Fix changelog files & add a simple validator#5199
Conversation
posix.mak
Outdated
| @echo "Validate changelog files (Do _not_ use REF in the title!)" | ||
| @echo " - Title line can't contain REF - it's already a link" | ||
| @echo " - After the title line an empty, separating line is expected" | ||
| @echo " - Which is supposed to be followed by a long description" |
There was a problem hiding this comment.
Not a fan of all this noise, could you please only print if if the validation fails?
FWIW, one-liners used to be ok. They would be displayed directly in the list, not as a jump link to a more detailed description. Example: the third point under "Library Changes" in the 2.072.0 changelog. I don't mind if we abandon that style, though. It messes with the numbering when jump links and one-liner are mixed (e.g., "Implementation of MurmurHash digest." is number 4 in the short list, but number 3 in the long list). Alternatively, allow one-liners, don't make jump links from them, allow links in them, and automatically put them at the end, behind the long-form entries. |
posix.mak
Outdated
| @echo " - Which is supposed to be followed by a long description" | ||
| for file in $$(find changelog -name '*.dd') ; do \ | ||
| echo "Testing: $$file"; \ | ||
| cat $$file | head -n1 | grep -nq '\$$' ; [ $$? -eq 0 ] && exit 1 ; \ |
There was a problem hiding this comment.
This just check for the dollar sign, right? I.e., it rejects all macros? This seems too restrictive. Macros that don't generate links should be allowed.
Done.
Imho if someone already makes the effort to open a new file it's not a big deal to add an additional line and it really helps the reader if there's a short example included.
Yet another reason to always require a description =)
Sure, the changelog tool could handle them properly and sort the list, but as mentioned above I don't think it's worth the effort. |
posix.mak
Outdated
|
|
||
| @echo "Validate changelog files (Do _not_ use REF in the title!)" | ||
| @for file in $$(find changelog -name '*.dd') ; do \ | ||
| echo "Testing: $$file"; \ |
posix.mak
Outdated
| @echo "Validate changelog files (Do _not_ use REF in the title!)" | ||
| @for file in $$(find changelog -name '*.dd') ; do \ | ||
| echo "Testing: $$file"; \ | ||
| cat $$file | head -n1 | grep -nqE '\$$\((REF|LINK2|HTTP|MREF)' ; [ $$? -eq 0 ] && \ |
There was a problem hiding this comment.
Instead of [ $$? -eq 0 ] && use ||
| @echo "Enforce space between binary operators" | ||
| grep -nrE "[[:alnum:]](==|!=|<=|<<|>>|>>>|^^)[[:alnum:]]|[[:alnum:]] (==|!=|<=|<<|>>|>>>|^^)[[:alnum:]]|[[:alnum:]](==|!=|<=|<<|>>|>>>|^^) [[:alnum:]]" $$(find . -name '*.d'); test $$? -eq 1 | ||
|
|
||
| @echo "Validate changelog files (Do _not_ use REF in the title!)" |
There was a problem hiding this comment.
This is consistent with all other checks. If you want to make the checks less noisy, I am happy to do so, but then we should do this for all checks and then in another PR.
There was a problem hiding this comment.
OK, then things should be fine for this PR at least. Thanks!
wilzbach
force-pushed
the
fix-changelog
branch
from
d7cff81 to
2e05f29
Compare
4 participants
It seems that not everyone was consistent with the changelog format - some people even left out the description :O
Well the changelog tool isn't called here directly, but on DAutoTest.
I tried to fix the changelog files & added a simple validator, s.t. we can at least automatically ensure that
$(REF)in the titleYes in theory the
changed.dcould have a flag---validate, but I would like to have this check enforced before next summer (PRs at tools have a tendency to hang in the queue).I don't think it's worth supporting this as (a) it's not the git commit format anymore and (b) a title shouldn't be that long anyways.
However in case this is wanted (and dlang/tools#220 is merged), the validator would need a small update.
CC @aG0aep6G
(This is a follow-up to #5192)