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

Improve changelog management #1058

Merged
merged 21 commits into from
Mar 14, 2024
Merged

Improve changelog management #1058

merged 21 commits into from
Mar 14, 2024

Conversation

Ndpnt
Copy link
Member

@Ndpnt Ndpnt commented Feb 23, 2024

  • Improve changelog validation:
    • Ensure that it can be parsed
    • Ensure that funder credit is present
    • Ensure that changes are present and well formed in the Unreleased section
  • Remove confusing Unreleased section title after releasing a version
  • Auto add link to related pull request of a released version
  • Allow to skip changelog validation by adding following section in the changelog:
## Unreleased [no-release]

 _Modifications made in this changeset do not alter its behavior or functionality from the perspective of the end user or other interacting systems. It does not add new features, does not enhance existing features, or alters the behavior of the software in a way that affects its use._

Remove dependency to external GitHub Action superfaceai/release-changelog-action

Fixes #1009

@MattiSG
Copy link
Member

MattiSG commented Mar 4, 2024

As discussed synchronously with @Ndpnt and @clementbiron, the current implementation has the following limitations:

  1. Contributors will receive a test failure until a maintainer adds the No Release label.
  2. Maintainers will need to manually trigger a test run after adding the No Release label.
  3. Increased vendor lock-in with GitHub.

An alternative could be to offer a line to copy-paste in the Unreleased section, in which the contributor certifies that, to the best of their knowledge, the contribution has no functional impact and should not trigger a release. Such an implementation would have the following limitations:

  1. Provide a green check mark for the check changelog check when the line is present, leaving more room for an accidental lack of release from a tired maintainer.
  2. Add a commit erasing the “Unreleased” section after each non-functional branch merge.
  3. Force an unusual action from contributors: copy-pasting a specific line from the CONTRIBUTING file.

Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

(partial review)

.github/workflows/changelog.yml Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
scripts/changelog/index.js Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
scripts/changelog/changelog.test.js Outdated Show resolved Hide resolved
@Ndpnt Ndpnt requested a review from MattiSG March 13, 2024 08:26
Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

Almost there, but the checks and the option syntax should be corrected.

.github/workflows/changelog.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
import ChangelogValidationError from './changelogValidationError.js';

export default class Changelog {
static FUNDER_REGEX = /^> Development of this release was(.+)$/m;
Copy link
Member

Choose a reason for hiding this comment

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

  • Follow the spec.
  • Ensure there is a closing full stop.
  • Why allow multiline?
Suggested change
static FUNDER_REGEX = /^> Development of this release was(.+)$/m;
static FUNDER_REGEX = /^> Development of this release was supported by (.+)\.$/;

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Multiline is required to have ^ and $ to match the begin/end of each line (not only begin/end of string). This is essential as we analyze the entire release content to verify the funder.
  • We have an issue with the specification regarding certain release funders formatted as follows: > Development of this release was made on a volunteer basis by …. Should we modify these types of funder sentences, or should we adjust the specification accordingly?

Copy link
Member

Choose a reason for hiding this comment

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

We should adjust the spec as volunteer contributions are an integral part of the contribution workflow.

Gotcha for the multiline, I had misunderstood this flag as valid for the match, not for the input.

scripts/changelog/index.js Outdated Show resolved Hide resolved
scripts/changelog/index.js Show resolved Hide resolved
Co-authored-by: Matti Schneider <matti@opentermsarchive.org>
Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

👏 🚀

@@ -4,7 +4,7 @@ import semver from 'semver';
import ChangelogValidationError from './changelogValidationError.js';

export default class Changelog {
static FUNDER_REGEX = /^> Development of this release was(.+)$/m;
static FUNDER_REGEX = /^> Development of this release (?:was supported by|was made on a volunteer basis by)(.+)\.$/m;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
static FUNDER_REGEX = /^> Development of this release (?:was supported by|was made on a volunteer basis by)(.+)\.$/m;
static FUNDER_REGEX = /^> Development of this release was (?:supported by|made on a volunteer basis by)(.+)\.$/m;

@Ndpnt Ndpnt merged commit bda5407 into main Mar 14, 2024
8 checks passed
@Ndpnt Ndpnt deleted the improve-changelog-management branch March 14, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add step in CI changelog checks to ensure changelog.md can be parsed
2 participants