|
|
Created on 2025-11-18.09:24:25 by clemens, last changed by remo.
| msg11993 (view) |
Author: remo |
Date: 2026-02-13.18:52:12 |
|
We continued work on this in the context of the February 2026 Sprint. After
additional reviews by Florian and Travis we merged the changes to the
documentation from https://github.com/aibasel/downward-markdown/pull/8.
In addition to documenting the new clang-format setup, we also restricted the
documented development setup from covering both Ubuntu 22.04 and 24.04 to only
the latter. This was motivated by the fact that getting the clang-18 on 22.04 is
not trivial and that there was no interest among developers to keep instructions
for 22.04 around.
The reviews revealed that the addition of clang-format-18 to the workflow is at
odds with the previously present clang-tidy-16. Because the upgrade from
clang-tidy-16 to clang-tidy-18 did not seem to cause any problems, we also
implemented the upgrade as part of this issue. Here is the merged pull request:
https://github.com/aibasel/downward/pull/279.
|
| msg11944 (view) |
Author: malte |
Date: 2026-01-14.13:22:16 |
|
Thanks for the answers! Works for me, but like I said in the previous message,
it would be nice for someone else to give the OK. (For example, the tox setup
that is mentioned in the docs didn't work for me last time I tried, so I cannot
verify the instructions given. And I lack the time right now to do much about that.)
|
| msg11943 (view) |
Author: remo |
Date: 2026-01-14.12:30:02 |
|
Simon and I replied to Malte's questions on GitHub, so we would be ready for another look.
|
| msg11942 (view) |
Author: malte |
Date: 2026-01-13.20:15:30 |
|
Looks great to me in general, but I left some comments.
Once you've answered, I
don't need to review this again, but I would like one of Florian, Gabi or
Clemens to give their OK before you merge.
|
| msg11939 (view) |
Author: simon |
Date: 2026-01-05.21:08:16 |
|
Remo and I continued to work on this issue today.
We made a pull request to the Basel private github repository
https://github.com/aibasel/downward-markdown/pull/8
This is ready to review now.
|
| msg11919 (view) |
Author: simon |
Date: 2025-12-12.17:02:15 |
|
Remo and I will tackle this next Thursday.
|
| msg11916 (view) |
Author: clemens |
Date: 2025-11-18.11:23:21 |
|
Indeed. To be honest, I didn't really look into the contents much after the topmost part which is about formatting comments (but I'm unsure to what extent clang-format can automatically check and enforce these conventions).
To do this right, it would probably take me more time than I can spare right now. I'll keep it on my radar, though (unless somebody beats me to it, which is absolutely fine by me).
|
| msg11915 (view) |
Author: malte |
Date: 2025-11-18.10:42:12 |
|
clang-format is about code formatting, but the majority of entries on the coding
conventions page isn't (how to organize the code into different directories,
namespaces etc.; semantic things like which kinds of references to use or how to
pass certain objects; anti-idioms like using size instead of empty). Only a
small minority of items are related to formatting. If you read through the page,
I'm sure you'll agree that it cannot be replaced by a reference to clang-
format.
The whitespace page is different. There I think most or all of the
stuff is now redundant. If some of that content should stay, it's probably not
enough to justify its own page, and any small remaining content could be moved
to the coding conventions page.
|
| msg11914 (view) |
Author: clemens |
Date: 2025-11-18.09:40:59 |
|
That depends on what we're going for. I don't have too much time to spare, unfortunately. If we want more than simply replace everything with a reference to clang-format, I don't see myself working on this anytime soon. I just stumbled over it and thought this would be the most reasonable place to take note.
|
| msg11913 (view) |
Author: malte |
Date: 2025-11-18.09:26:50 |
|
Do you want to prepare a pull request?
|
| msg11912 (view) |
Author: clemens |
Date: 2025-11-18.09:24:25 |
|
We've recently updated our code style tests to use clang-format rather than uncrustify. As far as I recall, with this change there is a strict formatting imposed on the code without much (if any) wiggle room. We should update our documentation on coding conventions (https://www.fast-downward.org/latest/for-developers/coding-conventions/) and whitespace guide (https://www.fast-downward.org/latest/for-developers/cpp-whitespace/) accordingly.
Today, they give a detailed overview of what we recommend and partially (used to) enforce. I think we can get rid of most of this unless we want to use this to give some rationale behind certain settings in our clang-format. If so, we should rather do this sooner than later while we can still remember why we chose the settings the way we did.
|
|
| Date |
User |
Action |
Args |
| 2026-02-13 18:52:12 | remo | set | messages:
+ msg11993 status: reviewing -> resolved |
| 2026-01-14 13:22:16 | malte | set | messages:
+ msg11944 |
| 2026-01-14 12:30:02 | remo | set | messages:
+ msg11943 |
| 2026-01-13 20:15:30 | malte | set | messages:
+ msg11942 |
| 2026-01-05 21:08:16 | simon | set | status: chatting -> reviewing messages:
+ msg11939 |
| 2025-12-12 17:02:15 | simon | set | nosy:
+ remo, simon messages:
+ msg11919 |
| 2025-11-18 11:23:21 | clemens | set | messages:
+ msg11916 |
| 2025-11-18 10:42:12 | malte | set | messages:
+ msg11915 |
| 2025-11-18 09:40:59 | clemens | set | messages:
+ msg11914 |
| 2025-11-18 09:26:50 | malte | set | messages:
+ msg11913 |
| 2025-11-18 09:24:25 | clemens | create | |
|