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

Remove Changelog page #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented May 2, 2022

As discussed in PR #65, the changelog page is not actively maintained and is out of date/out of sync with the actual changes which have been made, so we may as well remove it.

As discussed in PR 65, the changelog page is not actively maintained and is out of date/out of sync with the actual changes which have been made, so we may as well remove it.
@costdev
Copy link

costdev commented May 2, 2022

Since the changelog is out of sync/date and this PR proposes removing the changelog page altogether, does the lack of stated changes before and/or after mean the repo fails to meet GPLv2 for stating changes? Does the commit history count?

@ntwb
Copy link
Member

ntwb commented May 2, 2022

... fails to meet GPLv2 for stating changes? Does the commit history count?

I'm not a lawyer, but yeah, I think it should count 😏

That said, the changelog is published on w.org, so once merged we would need to delete this page from the site:

https://developer.wordpress.org/coding-standards/changelog/

I hadn't previously considered that we would also be deleting a published page, so might need to think about this some more....

@costdev
Copy link

costdev commented May 2, 2022

Out of curiosity, what do you think about automating the generation and update of changelog.md as a pre-commit hook?

LATEST=$(git log -n 20 --pretty="- %cd %s" --date=format:"%b %d %Y")
OLDER="[View the full commit history](https://github.com/WordPress/wpcs-docs/commits/master)"
printf "# Changelog\n## Latest changes\n$LATEST\n## Older changes\n$OLDER" > changelog.md
git add changelog.md

Which looks like this.

@jrfnl
Copy link
Member Author

jrfnl commented May 2, 2022

Out of curiosity, what do you think about automating the generation and update of changelog.md as a pre-commit hook?

LATEST=$(git log -n 20 --pretty="- %cd %s" --date=format:"%b %d %Y")
OLDER="[View the full commit history](https://github.com/WordPress/wpcs-docs/commits/master)"
printf "# Changelog\n## Latest changes\n$LATEST\n## Older changes\n$OLDER" > changelog.md

Which looks like this.

@costdev I appreciate the suggestion. Just looked at the output, but IMO that doesn't make for a very descriptive changelog and without links to the relevant PRs, IMO it's just as bad as the current one.

@costdev
Copy link

costdev commented May 2, 2022

Adding a descriptive commit subject line when committing directly to the main branch or when merging a PR would make the history more descriptive and remove the need for a link to PRs. A changelog doesn't need to link to PRs just as it doesn't need to link to the specific commit.

Also, I disagree that it's as bad as the current one, as this is at least up to date with the repo. While not descriptive, it's basically no worse than reading the repo's commit history.

@ntwb
Copy link
Member

ntwb commented May 2, 2022

I think a manually curated changelog is easiest, the repo can highlight the in-depth changes for the more curious.

Pre-commit hooks don't work well on GitHub, contributing here should quick and simple, quick edits here in the GitHUb.com site in the files directly, I even did that just now for #68

@jrfnl
Copy link
Member Author

jrfnl commented May 2, 2022

Adding a descriptive commit subject line when committing directly to the main branch or when merging a PR would make the history more descriptive and remove the need for a link to PRs. A changelog doesn't need to link to PRs just as it doesn't need to link to the specific commit.

Also, I disagree that it's as bad as the current one, as this is at least up to date with the repo. While not descriptive, it's basically no worse than reading the repo's commit history.

Sorry, but I don't agree.

  • The workflow is to always work via PRs (so no direct commits to the main branch),.
  • In the example output, there are currently basically two entries for every change - the merge commit + the PR commit.
  • Unless we change the workflow to use "Rebase & merge" for all merges, putting the burden of adding a "descriptive message" on merging a PR on maintainers is not lessening the maintenance burden, but raising it.
    This also makes it very prone to "human error", i.e, someone forgetting to do this, which would make the changelog the same as it is in the example, which is not helpful.
  • Fixes for typos and such should not need to be included in a changelog. Only functional changes should be listed. Listing everything just makes for a lot of noise for people to wade through.

A changelog doesn't need to link to PRs just as it doesn't need to link to the specific commit.

Except that the changelog is now based on the commits, so yes, it should.

Also, I disagree that it's as bad as the current one, as this is at least up to date with the repo. While not descriptive, it's basically no worse than reading the repo's commit history.

Well, I agree that it's more complete, but it's still not descriptive, which IMO make it very user unfriendly.

@costdev
Copy link

costdev commented May 2, 2022

TL;DR - Ultimately, automated changelog generation means the changelog can't be edited manually, as this would likely mean rebasing. So I agree with @ntwb that a manually curated changelog would be the better way to go to make sure the changelog is user friendly.


The workflow is to always work via PRs (so no direct commits to the main branch),.

Fair enough.

In the example output, there are currently basically two entries for every change - the merge commit + the PR commit.

That could be excluded from the automation by filtering out lines beginning with "Merge pull request", for example.

  • Unless we change the workflow to use "Rebase & merge" for all merges, putting the burden of adding a "descriptive message" on merging a PR on maintainers is not lessening the maintenance burden, but raising it.
  • This also makes it very prone to "human error", i.e, someone forgetting to do this, which would make the changelog the same as it is in the example, which is not helpful.

I agree. Although, this doesn't seem like any more burden than a manually curated changelog.

Fixes for typos and such should not need to be included in a changelog. Only functional changes should be listed. Listing everything just makes for a lot of noise for people to wade through.

I agree that this adds noise.

The GPLv2 states:

You must cause the modified files to carry prominent notices stating that you changed the files and the date of any change.

To be fair, this is extremely open to interpretation. Theoretically, "Changed file1.ext on date, file2.ext on date and file3.ext on date" would be compliant with the above. It doesn't appear to specify whether this excludes certain changes, although some resources online (and our experience of changelogs) would point to notable changes only.

Except that the changelog is now based on the commits, so yes, it should.

The changelog of any version controlled software is based on the commits. In an automated changelog based on commits, the commit subject line determines whether a link to more information is necessary.

Well, I agree that it's more complete, but it's still not descriptive, which IMO make it very user unfriendly.

I agree.

@jrfnl
Copy link
Member Author

jrfnl commented May 2, 2022

The GPLv2 states:

You must cause the modified files to carry prominent notices stating that you changed the files and the date of any change.

To be fair, this is extremely open to interpretation. Theoretically, "Changed file1.ext on date, file2.ext on date and file3.ext on date" would be compliant with the above. It doesn't appear to specify whether this excludes certain changes, although some resources online (and our experience of changelogs) would point to notable changes only.

That section of GPL does not apply.

That section is about forks of codebases where the code is modified from the original code. That section does not apply to original sources, which this repo is.

@jrfnl
Copy link
Member Author

jrfnl commented Jan 5, 2025

Would be good if we could get to a decision on this PR at some point this year....
/cc @WordPress/wpcs

Copy link
Member

@pento pento left a comment

Choose a reason for hiding this comment

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

I understand why it's nice to have a changelog, but the discussion is largely academic, given that in practice, the changelog just isn't used.

Copy link
Member

@dingo-d dingo-d left a comment

Choose a reason for hiding this comment

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

Seeing how the current changelog serves little to no purpose I will approve this PR.

Most of the changes made have good commit messages and discussions on the make blogs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants