Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize the format used in CHANGELOG and UPGRADE files #32840

Open
fancyweb opened this issue Jul 31, 2019 · 12 comments
Open

Normalize the format used in CHANGELOG and UPGRADE files #32840

fancyweb opened this issue Jul 31, 2019 · 12 comments

Comments

@fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Jul 31, 2019

The format used in those files greatly vary and different core team members have different expectations for them.

Sometimes it's on the case :

  • Added x
  • added x

Or on the final point :

  • Removed y
  • Removed y.

Or "the way" we write recurring themes such as deprecations :

  • Deprecated X, use Y.
  • The X class has been deprecated, please use Y
  • X is deprecated since Symfony 4.4, use Y instead.

etc. etc. etc.

What about deciding quickly on some rules that will cover most of the cases and write them down in the contributing doc ?

The gain is to stop wasting time on this subject : for reviewers so they can focus on the code and for contributors so they know how to write those messages.

@fancyweb

This comment has been minimized.

Copy link
Contributor Author

@fancyweb fancyweb commented Jul 31, 2019

That can also be applied in the code base for deprecation messages. It would really be easier to contribute for newcomers with help on these subjects.

@yceruto

This comment has been minimized.

Copy link
Member

@yceruto yceruto commented Jul 31, 2019

My suggestion would be to use this rule for UPGRADE/CHANGELONG files:

  • First letter in capital letters Added x
  • Always end with a dot Removed y.

And for deprecation messages this variant:

  • X is deprecated since Symfony 4.4, use Y instead.
@fancyweb

This comment has been minimized.

Copy link
Contributor Author

@fancyweb fancyweb commented Jul 31, 2019

I agree with you @yceruto.

The goal would be to make a list of common actions and their associated messages, such as :

Removed a class and it has no new alternative -> Removed X.
Removed a class and it has a new alternative -> Removed X, use Y instead.
Removed a method -> Removed X::method().
Made a class final -> Made X final.
Added a new method, class, interface, etc. etc. etc.

We will never cover everything but every new entry in this doc would make our messages more consistent and the review process less arbitrary and faster.

@ro0NL

This comment has been minimized.

Copy link
Contributor

@ro0NL ro0NL commented Aug 1, 2019

ideally we let fabbot do some normalization concerning dots, capitals, linebreaks, indenting etc.

@fabpot

This comment has been minimized.

Copy link
Member

@fabpot fabpot commented Aug 1, 2019

@fancyweb I've given up doing comments on this topic and I let people choose whatever they like (as this is not that important). But if we like to standardize on something, here are the rules I used at the beginning of this long story:

  • Lower case first word (which is a verb) -> added XXX
  • Never end with a dot
@fabpot

This comment has been minimized.

Copy link
Member

@fabpot fabpot commented Aug 1, 2019

I forgot to mention that the 2 rules I've given above are consistent with the commit messages I'm using.

@xabbuh

This comment has been minimized.

Copy link
Member

@xabbuh xabbuh commented Aug 1, 2019

I guess people started to use uppercase words and dots when sentences became longer or when entries had more than one sentence. Thus, if we wanted to go for consistency here, we will probably have to go with full sentences (i.e. uppercased first word and terminating dots).

@fancyweb

This comment has been minimized.

Copy link
Contributor Author

@fancyweb fancyweb commented Aug 2, 2019

ideally we let fabbot do some normalization concerning dots, capitals, linebreaks, indenting etc.

Yes! @fabpot Is that technically possible? And where is the source code? 😁

@nicolas-grekas

This comment has been minimized.

Copy link
Member

@nicolas-grekas nicolas-grekas commented Aug 2, 2019

Please create a simple command that would fix the files, I'll be able to add it to fabbot afterward.

Of course, please run this command on all existing files in the repo to validate it does its job correctly.

Ideally, we would also need a checker that checks that:

  • when a feature or a deprecation is submitted, CHANGELOG is updated
  • when a deprecation is submitted, both UPGRADE files are changed (current+next major)
  • when one UPGRADE is patched, both should be (did you forget to update ...)

The output of this command should be a series of messages + affected files, that we would then reuse in the fabbot report (see existing reports for inspiration, the format is pretty simple)

@ro0NL

This comment has been minimized.

Copy link
Contributor

@ro0NL ro0NL commented Aug 2, 2019

we should assume each entry can be multiline and needs to be e.g. re-indented as a whole

e.g. nested lists (https://github.com/symfony/symfony/blob/4.4/UPGRADE-5.0.md#serializer) and overall the before/after examples

perhaps KISS and reformat using some markdown parser, to guarantee markdown input/output :)

@lyrixx

This comment has been minimized.

Copy link
Member

@lyrixx lyrixx commented Aug 2, 2019

And please, forbid trailing spaces to avoid such diff

@A1rPun

This comment has been minimized.

Copy link

@A1rPun A1rPun commented Nov 21, 2019

Glad all upgrade- & change logs are in markdown. We could configure a markdown parser that let's us create rules on the different markdown elements. The reason for a markdown parser is to have control over all semantics. Just an idea, maybe it is overkill.

There are maybe more things you want to check or do on the upgrade- & change logs (if applicable).

  • capitalize the first letter of headings
  • run php-cs-fixer on code blocks
  • find dead links

There are markdown linters available but we could write a program in the likes of php-cs-fixer that could be dev-required when publishing a release.

$ md-style-fixer fix

Edit: It's a shame I skipped passed @ro0NL comment when I made this comment. I agree on using a markdown parser :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.