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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(python): standardize Python headers in the changelog #1390

Closed
wants to merge 4 commits into from

Conversation

dandhlee
Copy link
Contributor

Ported over existing entry from the Ruby-yoshi changelog updater. Only modifying for header changes and removing links in the version titles.

For any headers of size 2 or 3 for version change entries, they will be standardized to header size 2.

For any headers of size 2 or 3 for change type entries, they will be standardized to header size 3.

Fixes #1389 馃

@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Apr 14, 2022
@dandhlee dandhlee changed the title feat: standardize Python headers in the changelog feat(python): standardize Python headers in the changelog Apr 14, 2022
@dandhlee dandhlee marked this pull request as ready for review April 14, 2022 19:12
@dandhlee dandhlee requested a review from a team as a code owner April 14, 2022 19:12
@dandhlee dandhlee requested a review from a team April 14, 2022 19:12
Copy link
Member

@dazuma dazuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One caveat: This will affect EVERYONE who uses the python strategy, including non-Google users. Please make sure that is what you want. (For the Yoshi Ruby team, for example, we have a separate ruby-yoshi strategy subclass specific to our desired use case, and we modified that subclass rather than the main ruby strategy.)

src/strategies/python.ts Outdated Show resolved Hide resolved
@dandhlee
Copy link
Contributor Author

Thank you for taking a look! I think the inconsistent header problem would definitely benefit all users including non-Googlers.

As per dropping the version header link as well, that might be less desired for some folks but with the reasons I stated in the comments I think it is alright to drop the links to those. The information is still available on the GitHub page itself if folks were interested. But, if others feel strongly against this I'll be happy to drop it and only include the inconsistent header changes for all of Python.

@dazuma
Copy link
Member

dazuma commented Apr 14, 2022

I definitely defer to @bcoe on changes that could affect a large number of users.

@dandhlee
Copy link
Contributor Author

Please take a look again!

@dandhlee dandhlee requested a review from chingor13 April 14, 2022 22:57
@chingor13
Copy link
Contributor

chingor13 commented Apr 15, 2022

FWIW, the default changelog contents is being generated by conventional-changelog-writer. It's unclear from their docs and examples whether this is a feature or a bug.

cc @bcoe

edit: maybe this is a bug: conventional-changelog/conventional-changelog#867

@chingor13 chingor13 requested a review from bcoe April 15, 2022 17:10
@chingor13 chingor13 changed the title feat(python): standardize Python headers in the changelog fix(python): standardize Python headers in the changelog Apr 15, 2022
@bcoe
Copy link
Contributor

bcoe commented Apr 20, 2022

I think this is a feature which predates my work on the project, the idea being that features jump out more in the CHANGELOG than bug fixes.

Copy link
Contributor

@bcoe bcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather this change happen upstream in conventional-changelog-writer, than carry custom regex to rewrite the header size.

Care to make a PR there instead?

@bcoe
Copy link
Contributor

bcoe commented May 5, 2022

I've been talking to @dandhlee, we're working on an upstream fix.

@bcoe bcoe closed this May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Header levels are not standard for Python changelogs
4 participants