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

feat(release): support groupPreVersionCommand for release groups #27474

Merged

Conversation

jogelin
Copy link
Contributor

@jogelin jogelin commented Aug 16, 2024

Current Behavior

In Nx release configuration, the property preVersionCommand can be set only at the root level in the nx.json:

"release": {
    "version": {
        "preVersionCommand": "nx run-many -t build"
    }
},    

It is not possible to specify it per group meaning that if we want to release only one group, we have to build all releasable projects.

Expected Behavior

We should be able to specify preVersionCommand per group:

"release": {
    "groups": {
        "toolkits": {
            "projects": ["*-toolkit"],
            "version": {
                "groupPreVersionCommand": "nx run-many -t build -p \"*-toolkit\""
             }
        }
    }
},

In that way, we can align the list of projects related to the group and the list of projects to build

Copy link

vercel bot commented Aug 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Sep 16, 2024 0:13am

@jogelin

This comment was marked as resolved.

Copy link
Collaborator

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Given some feedback on X

@jogelin jogelin force-pushed the release-version-preVersionCommand-by-group branch from cdf725e to d254b0d Compare September 8, 2024 13:48
@jogelin jogelin marked this pull request as ready for review September 8, 2024 14:23
@jogelin jogelin requested review from a team as code owners September 8, 2024 14:23
Copy link
Collaborator

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Still pending the discussed updates

@JamesHenry JamesHenry changed the title feat(release): Allow preVersionCommand By Group feat(release): support groupPreVersionCommand for release groups Sep 10, 2024
@JamesHenry
Copy link
Collaborator

Updated the PR title to match what will be coming to the PR

@jogelin jogelin requested a review from JamesHenry September 11, 2024 13:20
}
```

The `groupPreVersionCommand` will run in addition of the global `preVersionCommand`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The `groupPreVersionCommand` will run in addition of the global `preVersionCommand`.
The `groupPreVersionCommand` will run in addition to the global `preVersionCommand`.

@@ -355,8 +356,16 @@ export async function createNxReleaseConfig(
);

// these options are not supported at the group level, only the root/command level
const rootVersionWithoutGlobalOptions = { ...rootVersionConfig };
const rootVersionWithoutGlobalOptions = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You left this underneath the comment saying it is not something which is supported at the group level, which doesn't make sense, but more broadly why are you creating a relationship between preVersionCommand and groupPreVersionCommand?

They are separate things, right now the way you have things it looks like if you set a preVersionCommand it will be run twice, once as a preVersionCommand and once as a groupPreVersionCommand?

Please remove this relationship

@@ -3313,6 +3331,7 @@ describe('createNxReleaseConfig()', () => {
"conventionalCommits": false,
"generator": "@nx/js:release-version",
"generatorOptions": {},
"groupPreVersionCommand": "nx run-many -t build",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This confirms what I said in my previous comment - this should not be set in this example, we do not want the command to be duplicated from global pre to group pre

@JamesHenry JamesHenry merged commit 71fe65f into nrwl:master Sep 17, 2024
6 checks passed
@JamesHenry
Copy link
Collaborator

Thank you @jogelin!

@jogelin jogelin deleted the release-version-preVersionCommand-by-group branch September 17, 2024 10:25
FrozenPandaz pushed a commit that referenced this pull request Sep 18, 2024
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants