Update release spec to only list changed packages #23

mcmire · 2022-07-27T05:35:07Z

This PR adds a new property to the Package object,
hasChangesSinceLatestRelease. This property is computed when a package
is read from a project by running a diff between the last-created Git
tag associated with that package and whatever the HEAD commit happens to
be, then checking to see whether any of the files changed belong to that
package. If they do, then hasChangesSinceLatestRelease is true,
otherwise it's false. This property is then used to filter the list of
packages that are placed within the release spec template when it is
generated.

There are a couple of things to consider here:

"the last-created Git tag associated with that package" — how do we
know this? How do we map a package's version to a tag? We have to
account for tags that were created by action-create-release-pr in the
past as well as the tags that this tool will create in the future. We
also have to know what kind of package this is — whether it's the root
package of a monorepo or a workspace package — because the set of
possible tags will differ. These differences are documented in
readMonorepoRootPackage and readMonorepoWorkspacePackage.
What happens if a repo has no tags? Then all of the packages in that
repo are considered to have changed, as they have yet to receive their
initial release, so they will all be included in the release spec
template.

Fixes #8.

mcmire · 2022-07-29T23:05:42Z

~~include-build-number-in-version needs to be merged into this and conflicts need to be resolved.~~ Done!

mcmire · 2022-09-08T22:24:45Z

Rebased!

mcmire · 2022-09-22T19:59:17Z

This has been rebased again and is now ready for review.

This PR adds a new property to the Package object, `hasChangesSinceLatestRelease`. This property is computed when a package is read from a project by running a diff between the last-created Git tag associated with that package and whatever the HEAD commit happens to be, then checking to see whether any of the files changed belong to that package. If they do, then `hasChangesSinceLatestRelease` is true, otherwise it's false. This property is then used to filter the list of packages that are placed within the release spec template when it is generated. There are a couple of things to consider here: * "the last-created Git tag associated with that package" — how do we know this? How do we map a package's version to a tag? We have to account for tags that were created by `action-create-release-pr` in the past as well as the tags that this tool will create in the future. We also have to know what kind of package this is — whether it's the root package of a monorepo or a workspace package — because the set of possible tags will differ. These differences are documented in `readMonorepoRootPackage` and `readMonorepoWorkspacePackage`. * What happens if a repo has no tags? Then all of the packages in that repo are considered to have changed, as they have yet to receive their initial release, so they will all be included in the release spec template.

src/package.ts

src/repo.ts

…t release

src/repo.ts

src/repo.test.ts

src/project.ts

Gudahtt · 2022-09-29T12:19:11Z

src/package.ts

+    generateMonorepoRootPackageReleaseTagNames(rootPackageVersion.toString());
+  const tagNameMatchingLatestRelease = [
+    ...expectedWorkspacePackageReleaseTagNames,
+    ...expectedRootPackageReleaseTagNames,


Perhaps it's worth printing a console warning if the fallback root tag is used? This should only happen in rare circumstances, like when using this tool for the first time on a monorepo that didn't previously have workspace tags (or at least not in the expected format).

A warning might hint that something is amiss if this happens outside of that scenario.

If we display a message to the user, I'd like it to be helpful and include a suggestion as to how they can resolve the problem, but I'm curious what that suggestion would be, because I think it differs based on whether this is the first time this tool is being used on a monorepo or not. If it is not the first time, then we can presume that all packages have been tagged properly, and so if the current release for any given package does not have the tag we expect, that is an anomaly and should be corrected. However, if it is the first time, then we can presume that no releases for any of the packages in the monorepo have tags at all, so in that case, if we say that the current release for a given package needs to be tagged properly, we might as well say that ALL releases for a given package need to be tagged properly.

So, it seems that either we need a way to tell whether this is the first time a user is using this tool, and only show this warning if it is not the first time, or we need to always show this warning, but offer another tool the user can use which will validate that all releases for all packages have proper tags and backfill them if necessary.

Thoughts?

Originally I was thinking the user would just want to know the fallback occurred, since as you point out it's not necessarily a problem or something that is actionable.

If we can do better, that would be great. I'll give this some thought.

If it is not the first time, then we can presume that all packages have been tagged properly, and so if the current release for any given package does not have the tag we expect, that is an anomaly and should be corrected.

Yes, agreed. We could even throw an error in this case, this should never happen. That would be an improvement. It's distinct from the suggestion I meant to make here though.

However, if it is the first time, then we can presume that no releases for any of the packages in the monorepo have tags at all, so in that case, if we say that the current release for a given package needs to be tagged properly, we might as well say that ALL releases for a given package need to be tagged properly.

If we're just starting to use this tool, I don't think we can make any assumptions about tags. We don't know what convention, if any, they were using before.

To be clear, I was suggesting that we warn in the case where we find a root monorepo tag but not a workspace tag. We proceed silently in that circumstance in this PR at the moment.

This seems worth highlighting so that the user understands it's happening, even just for auditability. For example, if there were unpublished changes in a workspace that predate the most recent root tag, they would be missed because of us using this fallback. I don't think this scenario is likely, nor do I think it makes sense necessarily to force the user to manually add tags. But calling out that it's happening might leave an audit trail to follow up on if it happens and someone is confused as to why.

Fair enough! Added warning in 565c2a5.

src/package.test.ts

Gudahtt · 2022-09-29T13:01:52Z

Generally this looks good! I left some comments on minor problems, and left one suggestion.

src/package.test.ts

Gudahtt

LGTM! one suggestion thread pending but it's non-blocking

Gudahtt

LGTM!

mcmire requested a review from a team as a code owner July 27, 2022 05:35

mcmire marked this pull request as draft July 29, 2022 17:16

mcmire self-assigned this Jul 29, 2022

mcmire force-pushed the include-build-number-in-version branch from c703a77 to d422085 Compare August 25, 2022 22:30

girlsavenue mentioned this pull request Aug 26, 2022

Add flow for monorepo w/ independent versions #6

Merged

mcmire mentioned this pull request Aug 26, 2022

Reformat root version to include the release no. #22

Merged

mcmire force-pushed the only-list-changed-packages branch from 6c124d0 to 34ccbf1 Compare September 8, 2022 22:07

Gudahtt linked an issue Sep 9, 2022 that may be closed by this pull request

Monorepo flow: When generating a release spec, only list changed packages #8

Closed

Base automatically changed from include-build-number-in-version to main September 22, 2022 19:34

mcmire force-pushed the only-list-changed-packages branch from 34ccbf1 to 3b55821 Compare September 22, 2022 19:58

mcmire marked this pull request as ready for review September 22, 2022 19:59

mcmire force-pushed the only-list-changed-packages branch from 3b55821 to 00b7396 Compare September 22, 2022 20:28

Fix atomic update issue

0c0c3c8

mcmire commented Sep 22, 2022

View reviewed changes

src/package.ts Outdated Show resolved Hide resolved

src/package.ts Outdated Show resolved Hide resolved

Gudahtt reviewed Sep 23, 2022

View reviewed changes

src/repo.ts Show resolved Hide resolved

src/repo.ts Outdated Show resolved Hide resolved

mcmire added 8 commits September 27, 2022 16:03

Throw if the project has tags but no tags can be found for the curren…

4986554

…t release

Remove rogue .only

8bde5e3

Sort Git tags by SemVer

5c13376

Remove unnecessary tests

d37bb88

Fix tests

d7a79ab

Merge branch 'main' into only-list-changed-packages

d1c8006

Fix tests

f8ab3f4

Fix tests, again

84f7c85

Gudahtt reviewed Sep 29, 2022

View reviewed changes

src/repo.ts Outdated Show resolved Hide resolved

Gudahtt reviewed Sep 29, 2022

View reviewed changes

src/repo.test.ts Outdated Show resolved Hide resolved

Gudahtt reviewed Sep 29, 2022

View reviewed changes

src/project.ts Show resolved Hide resolved

Gudahtt reviewed Sep 29, 2022

View reviewed changes

src/package.test.ts Outdated Show resolved Hide resolved

mcmire added 3 commits September 29, 2022 14:31

Remove duplicate test

156e8e8

Make description more accurate

8f7703f

Oops, remove WIP test

839054f

mcmire commented Sep 29, 2022

View reviewed changes

src/package.test.ts Outdated Show resolved Hide resolved

Make this more accurate as well

8190f5f

Gudahtt previously approved these changes Sep 29, 2022

View reviewed changes

Warn user if root package tag fallback is used

565c2a5

mcmire dismissed Gudahtt’s stale review via 565c2a5 September 30, 2022 21:23

Fix build

f1d539f

Gudahtt approved these changes Sep 30, 2022

View reviewed changes

mcmire merged commit 7d545df into main Sep 30, 2022

mcmire deleted the only-list-changed-packages branch September 30, 2022 22:27

Uh oh!

Update release spec to only list changed packages #23

Update release spec to only list changed packages #23

Uh oh!

Conversation

mcmire commented Jul 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcmire commented Jul 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcmire commented Sep 8, 2022

Uh oh!

mcmire commented Sep 22, 2022 • 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!

Uh oh!

Uh oh!

Gudahtt Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

mcmire Sep 29, 2022 • 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!

Gudahtt Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

Gudahtt Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

mcmire Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Gudahtt commented Sep 29, 2022

Uh oh!

Uh oh!

Gudahtt left a comment

Choose a reason for hiding this comment

Uh oh!

Gudahtt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcmire commented Jul 27, 2022 •

edited

Loading

mcmire commented Jul 29, 2022 •

edited

Loading

mcmire commented Sep 22, 2022 •

edited

Loading

mcmire Sep 29, 2022 •

edited

Loading