Allow using existing Keep-A-Changelog files for release notes #18

weierophinney · 2020-07-31T16:21:35Z

This patch modifies the workings of the ReleaseCommand slightly.

It now accepts a new argument Laminas\AutomaticReleases\Changelog\ReleaseChangelog $releaseChangelog. The ReleaseChangelog interface is intended to prepare a changelog in Keep-A-Changelog format for release (generally by adding the release date), commit it, and push it back to the origin repository.

To accomplish this, I created:

CommitFile, which is intended for, you guessed it, committing a file to the local checkout. One implementation was provided, CommitFileViaConsole.
Laminas\AutomaticReleases\Changelog\ReleaseChangelogViaKeepAChangelog, which uses tooling from phly/keep-a-changelog to add the release date to the CHANGELOG.md file, and then a CommitFile and Push instance to commit and push the changes. If the file does not exist, is in a different format, or has no changes necessary, it acts as a no-op.

To allow more ways to generate the release text, I added two additional arguments to the CreateReleaseText interface: BranchName $sourceBranch and string $repositoryPath. Additionally, I added a canCreateReleaseText(): bool method that accepts the same arguments as __invoke(); this allows testing to see if a given implementation will be able to create the release text.

From there, I added:

CreateReleaseTextViaKeepAChangelog. It's canCreateReleaseText() method returns false if the file does not exist, is malformed, or does not have an entry corresponding to the version. Otherwise, the __invoke() method extracts the changelog entry using the phly/keep-a-changelog ChangelogParser.
AppendingCreateReleaseTextAggregate, which accepts a list of CreateReleaseText instances. It filters for those capable of creating release text, and then concatenates them with \n\n-----\n\n.

When creating the ReleaseCommand in bin/console.php, I now create an AppendingCreateReleaseTextAggregate that uses a CreateReleaseTextViaKeepAChangelog instance and a CreateReleaseTextThroughChangelog instance.

Fixes #5

composer.json

src/Git/CommitFile.php

src/Changelog/ReleaseChangelogAndFetchContents.php

src/Changelog/ReleaseChangelogEvent.php

src/Changelog/UseKeepAChangelogEventsToReleaseAndFetchChangelog.php

weierophinney · 2020-08-04T20:49:45Z

It looks like the failing unit test is likely due to a missing git author/email, but it's hard to determine. Tests pass fine locally, which is why I'm inferring it's an environment issue.

weierophinney · 2020-08-04T20:51:14Z

It looks like the failing unit test is likely due to a missing git author/email, but it's hard to determine.

Verified: https://github.com/laminas/automatic-releases/pull/18/checks?check_run_id=946634877#step:8:68

Do you want me to add the git author/email to the travis env as part of this patch?

weierophinney · 2020-08-04T20:52:44Z

@Ocramius Okay, ready for another round of review. 😄

weierophinney

A few comments on implementation details...

test/unit/Changelog/ReleaseChangelogViaKeepAChangelogTest.php

test/unit/Github/CreateReleaseTextViaKeepAChangelogTest.php

src/Github/AppendingCreateReleaseTextAggregate.php

src/Changelog/ReleaseChangelogViaKeepAChangelog.php

Ocramius

Giving my early LGTM here.

The remaining comments are:

naming (ZF habits are hard to let go of :D )
testing (in CI, the GIT_* environment variables are not defined, so all mock/stub repos need to be configured with user/email before committing)
some minor adjustments around assertions
did you try it out manually somewhere?

bin/console.php

composer.json

Ocramius · 2020-08-05T21:07:52Z

src/Changelog/ReleaseChangelog.php

+use Laminas\AutomaticReleases\Git\Value\BranchName;
+use Laminas\AutomaticReleases\Git\Value\SemVerVersion;
+
+interface ReleaseChangelog


This name is not clear: is it supposed to be "commit release changelog" perhaps? Since its signature is : void, I suppose it does something

Yeah, CommitReleaseChangelog makes sense - I was thinking "release" as a verb here, but we also have a concept of "a release" throughout. If that name makes sense, I'll update.

src/Changelog/ReleaseChangelogViaKeepAChangelog.php

Ocramius · 2020-08-05T21:11:40Z

src/Changelog/ReleaseChangelogViaKeepAChangelog.php

+            $repositoryDirectory,
+            $sourceBranch,
+            'CHANGELOG.md',
+            sprintf('%s readiness', $versionString)


1.2.3 readiness doesn't sound like an awesome commit text: can we use something that explains that we did just provide a changelog text?

src/Github/CreateReleaseTextViaKeepAChangelog.php

test/unit/Changelog/ReleaseChangelogViaKeepAChangelogTest.php

test/unit/Git/CommitFileViaConsoleTest.php

test/unit/Github/CreateReleaseTextViaKeepAChangelogTest.php

This is "manual yesting" (yuck!) for laminas/automatic-releases#18

To be used as an alternate branch when a CHANGELOG file is available. Requires switching from ocramius/package-versions to composer/package-versions-deprecated. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

…ELOG and fetch CHANGELOG contents This patch modifies the workings of the `ReleaseCommand` slightly. Instead of depending on the `CreateReleaseText` interface, it now depends on a new `Laminas\AutomaticReleases\Changelog\ReleaseChangelogAndFetchContents` interface. This new interface consumes a `ReleaseChangelogEvent`, which stores a variety of runtime-determined values (console input/output, milestone, repository name and path, etc.). Implementors of this interface pull values from the event that they might use to fetch or create the changelog and return it. If they are incapable of doing so, they return null. I have created three implementations: - `UseKeepAChangelogEventsToReleaseAndFetchChangelog`, which checks to see if there is a `CHANGELOG.md` file in the repository. If not, it returns null immediately. Otherwise, it sets the release date in the changelog for the given version, commits the file (more on that later), and pushes the changes to the release branch. It then returns the discovered changelog contents - `CreateChangelogViaMilestone` pulls data from the event for use with the original `CreateReleaseText` implementation, and decorates such an instance. - `ReleaseChangelogAndFetchContentsAggregate` takes a list of `ReleaseChangelogAndFetchContents` instances, and returns the first changelog discovered. If none return one, it generates a basic one using just the repository name and release version. I have updated `bin/console.php` to create a `ReleaseChangelogAndFetchContentsAggregate` instance composing each of the other two types, with the keep-a-changelog version being first. To assist in how the `UseKeepAChangelogEventsToReleaseAndFetchChangelog` implementation works, I created `Laminas\AutomaticReleases\Git\CommitFile`, which describes a commit operation on a single file. `Laminas\AutomaticReleases\Git\CommitFileViaConsole` does a `git add` operation followed by a `git commit` operation on the file. Currently, no tests; I needed to play with architecture to figure out a workable approach first. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

While composer/package-versions-deprecated is deprecated with Composer 2, we will be using it for the forseeable future. As such, it's an expected issue, and should be suppressed. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

- Updates to phly/keep-a-changelog 2.8, which adds typed accessors to its `ChangelogEntry` class, which is used internally here, and for which psalm was raising errors due to inability to determine types. - Updates `bin/console.php` to create a `$createCommitText` variable assigned to a `CreateReleaseTextThroughChangelog` instance. This instance is then composed in the `$createReleaseText` array, and used directly for the `CreateMergeUpPullRequest`, where that changelog type makes sense as part of a commit message. - Adds `@psalm-*` annotations to better detail properties, arguments, and variables where necessary. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Demonstrates the behaviors of `ReleaseChangelogAndFetchContentsAggregate`. Also modifies the return type to only `string` (instead of being nullable), as the aggregate will always return a string, which follows PHP covariance rules. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

…elog Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

As noted during review, git honors any environment variables defining the current author and email, so there's no need to pass them in. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Removes input/output awareness from the event; we can use dummy instances when triggering the event later, as the input and output are not consulted. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Ensures that we commit against the correct branch. `CommitFileViaConsole` is now updated to checkout the given branch before adding and committing the file. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Describes classes that can prepare a changelog file for release. One implementation: ReleaseChangelogViaKeepAChangelog, which uses phly/keep-a-changelog tools to determine (a) if there is a changelog to release, and then (b) updates the date in it, commits the file, and pushes it. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Updates `CreateReleaseText` to: - Add a `BranchName $sourceBranch` argument - Add a `string $repositoryDir` argument - Add a new `canCreateReleaseText()` method with identical arguments, returning a boolean. The method should be called before invoking the implementation to create release text. After making those changes, updated `CreateReleaseTextThroughChangelog` to add the arguments and new method, and added a test for the new method. Finally, adds a new class, `CreateReleaseTextThroughChangelog`, which uses the phly/keep-a-changelog `ChangelogParser` to determine if (a) it can locate a CHANGELOG.md file and changelog entry matching the release version, and then (b) pull it and return it. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

…eateReleaseText implementation Signed-off-by: Matthew Weier O'Phinney <[email protected]>

…back to CreateReleaseText Adds a new dependency to the `ReleaseCommand`, `ReleaseChangelog $releaseChangelog`, and reverts the `$createChangelogText` argument to the `CreateReleaseText` type. Internally, it now executes the `$releaseChangelog` prior to calling `$createReleaseText`. In `bin/console.php`, we now create an `AppendingCreateReleaseTextAggregate` to inject as the `$createReleaseText` argument of `ReleaseCommand`, using each of `CreateReleaseTextViaKeepAChangelog` and `CreateReleaseTextThroughChangelog` instances, in that order. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Adds lcobucci/clock as a dependency, and updates `ReleaseChangelogViaKeepAChangelog` to accept a `Clock` instance as a dependency. This value is then used to obtain the release date. In the console tool itself, a `SystemClock` is provided; for testing, we can use a `FrozenClock` instance. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Initializes the CHANGELOG.md file, and adds an entry for laminas#18. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

…eMultipleReleaseTexts` More descriptive, less "patterny" name. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

- Renames "strategies" to "releaseTextGenerators" everywhere, and "strategy" to "generator". - Updates typehint for `$releaseTextGenerators` property to `@psalm-param non-empty-list<CreateReleaseText>` - Instead of declaring `$changelog` a non-empty-string, asserts it. - Adds `static` declaration to arrow function to fix CS issues. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Since "release" is used primarily as a noun in this package, the naming was ambiguous because it _does_ something; as such, added the verb `Commit` to the naming of the interface and implementations. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

In addition to the subject `{version} readiness`, the commit text now also includes the text "Updates the CHANGELOG.md to set the release date." Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Use `Assert::notEmpty()` instead of `@psalm-var non-empty-string` to ensure we get back a non-empty changelog. Additionally, added the same check in `canCreateReleaseText()` so that it returns false if the release text returned is empty. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

.github/workflows/psalm.yml

Follows same workflow as `PushViaConsole` tests. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

Ocramius

LGTM 🚢

weierophinney added BC Break Enhancement Review Needed Unit Test Needed Work In Progress labels Jul 31, 2020

weierophinney force-pushed the feature/keep-a-changelog branch from 994a076 to 3b82bd6 Compare July 31, 2020 16:33

Ocramius requested changes Aug 1, 2020

View reviewed changes

Ocramius mentioned this pull request Aug 2, 2020

Update Changelog when releasing doctrine/automatic-releases#17

Closed

weierophinney force-pushed the feature/keep-a-changelog branch 2 times, most recently from 2ab6074 to 05fc79a Compare August 4, 2020 20:46

weierophinney removed Unit Test Needed Work In Progress labels Aug 4, 2020

weierophinney changed the title ~~[WIP] Allow using existing Keep-A-Changelog files for release notes~~ Allow using existing Keep-A-Changelog files for release notes Aug 4, 2020

weierophinney commented Aug 4, 2020

View reviewed changes

Ocramius approved these changes Aug 5, 2020

View reviewed changes

Ocramius mentioned this pull request Aug 5, 2020

Feature/keep a changelog Ocramius/automatic-releases#7

Merged

Ocramius added a commit to Ocramius/just-trying-automated-releases that referenced this pull request Aug 5, 2020

Switch to "ocramius/[email protected]" to try out things

d28d55f

This is "manual yesting" (yuck!) for laminas/automatic-releases#18

This was referenced Aug 5, 2020

Switch to "ocramius/[email protected]" to try out things Ocramius/just-trying-automated-releases#17

Merged

Whenever a CHANGELOG.md release date is set, also create a new "next release" block with the TBD in the date #33

Closed

weierophinney added 8 commits August 6, 2020 11:19

qa: add phly/keep-a-changelog as a dependency

a875b8b

To be used as an alternate branch when a CHANGELOG file is available. Requires switching from ocramius/package-versions to composer/package-versions-deprecated. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

qa: adapted ReleaseCommandTest to mirror changes to ReleaseCommand

3e2e89e

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

qa: apply phpcs fixes

c98f47d

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

qa: unit tests for CreateChangelogViaMilestone class

5e23df3

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

weierophinney added 18 commits August 6, 2020 11:19

qa: adds unit tests for CommitFileViaConsole

11bdaae

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

qa: adds unit tests for UseKeepAChangelogEventsToReleaseAndFetchChang…

a8c3af2

…elog Signed-off-by: Matthew Weier O'Phinney <[email protected]>

qa: apply CS fixes and psalm advice

3349594

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

fix: remove author awareness from CommitFile and ReleaseChangelogEvent

d2707b1

As noted during review, git honors any environment variables defining the current author and email, so there's no need to pass them in. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

refactor: remove input/output awareness from ReleaseChangelogEvent

b6bf9d5

Removes input/output awareness from the event; we can use dummy instances when triggering the event later, as the input and output are not consulted. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

refactor: accept a branch name when committing a file

3a40424

Ensures that we commit against the correct branch. `CommitFileViaConsole` is now updated to checkout the given branch before adding and committing the file. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

refactor: creates AppendingCreateReleaseTextAggregate as aggregate Cr…

dba9e17

…eateReleaseText implementation Signed-off-by: Matthew Weier O'Phinney <[email protected]>

docs: adds initial CHANGELOG.md file.

9641022

Initializes the CHANGELOG.md file, and adds an entry for laminas#18. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

refactor: rename AppendingCreateReleaseTextAggregate to `Concatenat…

b7a888f

…eMultipleReleaseTexts` More descriptive, less "patterny" name. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

refactor: assign text CHANGELOG.md to a class constant

a5deb57

Signed-off-by: Matthew Weier O'Phinney <[email protected]>

feat: more commit text when committing changelog

d9fe978

In addition to the subject `{version} readiness`, the commit text now also includes the text "Updates the CHANGELOG.md to set the release date." Signed-off-by: Matthew Weier O'Phinney <[email protected]>

weierophinney force-pushed the feature/keep-a-changelog branch from 05fc79a to 8be5ae2 Compare August 6, 2020 16:19

Ocramius reviewed Aug 6, 2020

View reviewed changes

.github/workflows/psalm.yml Outdated Show resolved Hide resolved

weierophinney force-pushed the feature/keep-a-changelog branch 2 times, most recently from 52917fb to fa98694 Compare August 6, 2020 16:33

qa: refactor how CommitFileViaConsoleTest initializes dummy repo

7d9c597

Follows same workflow as `PushViaConsole` tests. Signed-off-by: Matthew Weier O'Phinney <[email protected]>

weierophinney force-pushed the feature/keep-a-changelog branch from fa98694 to 7d9c597 Compare August 6, 2020 16:35

Ocramius removed the BC Break label Aug 6, 2020

Ocramius self-assigned this Aug 6, 2020

Ocramius added this to the 1.1.0 milestone Aug 6, 2020

Ocramius approved these changes Aug 6, 2020

View reviewed changes

Ocramius merged commit 0bd9b0e into laminas:1.1.x Aug 6, 2020

weierophinney deleted the feature/keep-a-changelog branch August 6, 2020 16:56

Uh oh!

Allow using existing Keep-A-Changelog files for release notes #18

Allow using existing Keep-A-Changelog files for release notes #18

Uh oh!

Conversation

weierophinney commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

weierophinney commented Aug 4, 2020

Uh oh!

weierophinney commented Aug 4, 2020

Uh oh!

weierophinney commented Aug 4, 2020

Uh oh!

weierophinney left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ocramius left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Ocramius Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

weierophinney Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Ocramius Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

weierophinney Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ocramius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weierophinney commented Jul 31, 2020 •

edited

Loading

Ocramius left a comment •

edited

Loading