Add CommitFunctions and update config with defaults by rohit-gohri · Pull Request #768 · changesets/changesets

rohit-gohri · 2022-03-05T08:59:41Z

Follow up of #735

Closes #688
Closes #285

Made commit option similar to changelog option but with a default template when "true" is passed (keeping it backward compatible).

~~This currently doesn't allow disabling commit for one of add/version. Maybe that can be added too if the respective function returns null or doesn't exist?~~
If one of the functions doesn't exist then we will not commit for that only.

changeset-bot · 2022-03-05T08:59:43Z

🦋 Changeset detected

Latest commit: 8b855ed

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
@changesets/types	Major
@changesets/cli	Minor
@changesets/apply-release-plan	Major
@changesets/config	Major
@changesets/assemble-release-plan	Patch
@changesets/changelog-git	Patch
@changesets/changelog-github	Patch
@changesets/get-dependents-graph	Patch
@changesets/get-release-plan	Patch
get-workspaces	Patch
@changesets/git	Patch
@changesets/parse	Patch
@changesets/pre	Patch
@changesets/read	Patch
@changesets/release-utils	Patch
@changesets/write	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codesandbox-ci · 2022-03-05T09:00:05Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8b855ed:

Sandbox	Source
Vanilla	Configuration

packages/types/src/index.ts

packages/config/src/index.ts

Andarist · 2022-03-05T13:14:02Z

packages/apply-release-plan/src/index.ts

+    if (
+      typeof possibleCommitFunc.getAddLine === "function" &&
+      typeof possibleCommitFunc.getVersionLine === "function"
+    ) {
+      getCommitFuncs = possibleCommitFunc;
+    } else {
+      throw new Error("Could not resolve commit generation functions");
+    }


I guess we can start from requiring both to exist - but I wonder if there are people only wanting one of those to kick in 🤔

We can treat not existing as commit: false for add/version separately.

Added support for partial functions. If only one of it is supplied then commit will be done for that command only

Andarist · 2022-03-05T13:24:21Z

packages/cli/src/commit/index.ts

+    Releases:
+    ${releasesLines}
+
+    [skip ci]


What if we would remove this from here and instead add options to this function and only render this based on smth like options.versionSkipCI === true. This way we could "translate" current commit: true to ["@changesets/cli/commit", { versionSkipCI: true }].

This would be quite nice because one could more easily extend the built-in function without having to strip [skip ci] from it if they don't want it. This option would go away in Changesets v3 (if that ever gets published) and would only be supported now to avoid breaking changes.

This is a great idea, adds back skipCI but as a different option

I added for both versionSkipCI and addSkipCI, with versionSkipCI as true for the true config

addSkipCI seems like a weird option name now that I have added it, WDYT about just having skipCI and possible values being 'version' | 'add' | boolean?

that works too, I don't mind any of those solutions - I agree that the current names are a little bit weird but since I'd like to remove this in the future anyway... I don't care that much about this :p

Changed to "skipCI": "version" | "add" | boolean as the option.

Andarist · 2022-03-05T13:25:31Z

packages/cli/src/commit/index.ts

+
+export default defaultCommitFunctions;
+
+export function getCommitFuncs(


is this actually used anywhere? or just in tests?

It is used in the add function. I wanted to also use it in apply-release-plan but couldn't figure out how that import would work

I think that it would make sense to introduce a breaking change to the @changesets/apply-release-plan (that would not be a breaking change for the Changesets itself) and just move out the committing logic from it. Based on its return value the "version commit" could be conditionally created within @changesets/cli.

This would make both commit types be performed by a single package - which I think would be a good thing for the codebase.

Moved to the cli 🙂

rohit-gohri · 2022-03-06T06:29:52Z

All done from my side, the only thing remaining is the name of the skipCI options. See this comment

packages/types/src/index.ts

packages/cli/src/commit/commit.test.ts

Andarist · 2022-03-06T13:09:01Z

packages/cli/src/commands/version/index.ts

+    while (newTouchedFilesArr.length > 0) {
+      let file = newTouchedFilesArr.shift();
+      await git.add(path.relative(cwd, file!), cwd);
+    }


we don't need to touch this here but it's probably better to construct a single call to git add since stuff like git add file1.js file2.js ... is a totally valid thing to do

The add function in @changesets/git only accepts one path so I'm going to leave this for now.

packages/apply-release-plan/src/index.test.ts

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Andarist · 2022-03-06T13:47:02Z

The PR is almost ready to be shipped, great work! Once all remaining items (mostly nits really) get addressed I will rethink the rollout strategy as we need to consider how this affects the version of the involved packages and how to ensure that this doesn't break any existing consumers.

LeviticusMB · 2022-03-26T10:04:03Z

I would love to see this PR merged as soon as possible. What's holding it back?

Really looking forward to being able to turn auto-commit back on without getting that docs(changeset) prefix (which makes no sense when the actual change is just one commit) in the git log.

docs/config-file-options.md

packages/types/src/index.ts

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Andarist · 2022-03-27T10:37:23Z

What's holding it back?

Lack of the free time of maintainers 😉

Andarist · 2022-03-27T12:24:37Z

@rohit-gohri thank you very much for your contributions and bearing with me. People like you keep OSS projects afloat 🚀

LeviticusMB · 2022-03-27T18:38:03Z

Thanks a lot guys, really awesome! Works as advertised for me.

However, it does look like config/schema.json was not updated to reflect the change?

Andarist · 2022-03-27T21:31:00Z

However, it does look like config/schema.json was not updated to reflect the change?

Yeah, we forgot about it - PR with an update would be appreciated.

rohit-gohri added 5 commits March 5, 2022 14:23

Add CommitFunctions and update config with defaults

25ca057

Add changeset

379bcca

add checks and types for new commit option

b91bf60

moved default to cli

4220d62

update tests

5b50c75

rohit-gohri mentioned this pull request Mar 5, 2022

Update Commit config to separate add/version and add back skipCi #735

Closed

fix build

1f8d9b6

Andarist reviewed Mar 5, 2022

View reviewed changes

packages/types/src/index.ts Outdated Show resolved Hide resolved

Andarist reviewed Mar 5, 2022

View reviewed changes

packages/config/src/index.ts Outdated Show resolved Hide resolved

Andarist reviewed Mar 5, 2022

View reviewed changes

rohit-gohri added 3 commits March 5, 2022 21:37

rename commit fns

ffe53bd

add option for skipCI

bf60437

update tests

95b77af

rohit-gohri requested a review from Andarist March 5, 2022 17:13

rohit-gohri added 5 commits March 6, 2022 10:11

move commit logic to cli

37392bf

update tests for commit

2943889

allow disabling commit for only one of add/version

268e9aa

update docs

e191d3e

update tests

3001433

Andarist reviewed Mar 6, 2022

View reviewed changes

packages/types/src/index.ts Outdated Show resolved Hide resolved

Andarist reviewed Mar 6, 2022

View reviewed changes

packages/cli/src/commit/commit.test.ts Outdated Show resolved Hide resolved

Andarist reviewed Mar 6, 2022

View reviewed changes

packages/apply-release-plan/src/index.test.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

f326716

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

rohit-gohri added 2 commits March 6, 2022 19:18

remove unnecessary partial

3f3d3cb

change option name

90ebcac

Andarist reviewed Mar 26, 2022

View reviewed changes

docs/config-file-options.md Outdated Show resolved Hide resolved

Andarist reviewed Mar 26, 2022

View reviewed changes

docs/config-file-options.md Outdated Show resolved Hide resolved

Andarist reviewed Mar 26, 2022

View reviewed changes

packages/types/src/index.ts Outdated Show resolved Hide resolved

Andarist reviewed Mar 26, 2022

View reviewed changes

packages/types/src/index.ts Outdated Show resolved Hide resolved

rohit-gohri and others added 3 commits March 27, 2022 15:14

Apply suggestions from code review

0151fa6

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Small tweaks after review

2c7d182

Tweak changesets

8b855ed

Andarist merged commit c87eba6 into changesets:main Mar 27, 2022

github-actions bot mentioned this pull request Mar 27, 2022

Version Packages #777

Merged

rohit-gohri mentioned this pull request Apr 20, 2022

Add commit generator options to JSON schema #791

Closed


		export default defaultCommitFunctions;

		export function getCommitFuncs(

Conversation

rohit-gohri commented Mar 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Mar 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

codesandbox-ci bot commented Mar 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rohit-gohri commented Mar 6, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Andarist commented Mar 6, 2022

Uh oh!

LeviticusMB commented Mar 26, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Andarist commented Mar 27, 2022

Uh oh!

Andarist commented Mar 27, 2022

Uh oh!

LeviticusMB commented Mar 27, 2022

Uh oh!

Andarist commented Mar 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rohit-gohri commented Mar 5, 2022 •

edited

Loading

changeset-bot bot commented Mar 5, 2022 •

edited

Loading

codesandbox-ci bot commented Mar 5, 2022 •

edited

Loading