tophat / monodeploy

:monorail: Like semantic-release and Lerna, but for Yarn modern workspaces
https://tophat.github.io/monodeploy/
Apache License 2.0
106 stars 7 forks source link

Attach tags to the "release" commit that modifies the package.jsons #402

Closed fmal closed 2 years ago

fmal commented 3 years ago

This is a follow-up issue after discussion in https://github.com/tophat/monodeploy/issues/292#issuecomment-879216394.

Currently monodeploy adds the release tags on the commit with the actual feature change / bug fix. In some cases (such as conventional changelog config that uses patch as a default strategy for other commit types) it's more desirable to have tags on the "release" commit that modifies the package.jsons.

This is also what lerna seems to be doing: https://github.com/lerna/lerna/commit/4582c476e07dddddd6b2e3ab6e7f52c1f9eed59a

noahnu commented 3 years ago

We can definitely add this, however to entertain an alternative (we can do either, or both):

What if we skipped commits which contained "[skip ci]" in the commit message? This could also be customized via some sort of commit ignore pattern, similar to the changesetIgnorePatterns config option.

fmal commented 3 years ago

@noahnu that's a great idea to have a way to skip certain commits when calculating the version strategy, i.e. have a pattern to ignore commits starting with chore: release or containing [skip ci]. It would definitely solve my case. It would make sense for me to move the release tags to the commit modifying the versions, but as long as there is some option to ignore certain commits when calculating the release strategy, that's even more flexible 😄

noahnu commented 3 years ago

@fmal The latest version of monodeploy should help. In your config file:

commitIgnorePatterns: ['\\[skip ci\\]'],

Docs: https://tophat.github.io/monodeploy/configuration/#schema-option-commitIgnorePatterns (due to limitations with the ajv schema validation, the config option only accepts strings not regexp objects)

markandrus commented 2 years ago

I have a Slack integration which posts the new packages (well, git tags) which were released. It is sometimes confusing to follow the git tag into GitHub and see that the package.json shows a different version number. So I would also be interested in this feature 👍

noahnu commented 2 years ago

This should be fairly trivial to do. We'd just move this down to L39: https://github.com/tophat/monodeploy/blob/b03a9de1372cd35c3799752b5150f542b2909481/packages/publish/src/commitPublishChanges.ts#L21

I wonder if this should become the default behaviour 🤔 . To prevent a breaking change for the moment, we can add some "releaseStrategy" / "tagStrategy" option

markandrus commented 2 years ago

@noahnu that sounds reasonable! But I need to look at what gitPushTags does to fully understand the suggestion.

This does remind me, though: I am currently disabling monodeploy from automatically pushing. Instead, I iterate through the changeset and push tags myself, one-by-one, before pushing the release commit. This is because I have a GitHub integration that responds to new git tag events, and GitHub will not issue a webhook if more than 3 tags are pushed at a time. This feels like a separate issue, but the method name gitPushTags reminded me of it.

noahnu commented 2 years ago

Would something like https://github.com/tophat/monodeploy/issues/415 help at all?

markandrus commented 2 years ago

@noahnu I suppose a "fixed" mode could solve this case, since everything gets bumped together, but I think independent mode makes more sense for my projects in the long run. I believe it should minimize unnecessary updates.


BTW, I took a look at commitPublishChanges and I believe that we'd need to

This should result in

  1. Commit
  2. Tag
  3. Publish

Does my proposal make sense? Allowing to configure this behavior in a clean way may be tricky.

noahnu commented 2 years ago

Yes that makes sense.

To support configuration options, we could have createReleaseGitTags tag the previous commit if necessary. Though I'm starting to think we don't need to support configuration options here. We can make this the new behaviour. It'll be a breaking change, and we can batch it with some of the other breaking changes I had in mind.

noahnu commented 2 years ago

This will be out in monodeploy v3 which I'm working on: https://github.com/tophat/monodeploy/pull/452

markandrus commented 2 years ago

Awesome! 🎉

noahnu commented 2 years ago

Fixed in pre-release: https://github.com/tophat/monodeploy/releases/tag/monodeploy%403.0.0-rc.1 (wouldn't recommend using the pre-release though, will do a proper release as I wrap up some other changes)