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(core): add support for tags with (print-)affected(:*) and run-many #10085

Merged
merged 3 commits into from
Apr 6, 2023

Conversation

fguitton
Copy link
Contributor

@fguitton fguitton commented May 1, 2022

⚠️ Edit: The behaviour of this pull request has changed to address feedbacks. See #10085 (comment) for more information

Current Behavior

When running the (print-)affected(:*) command you get all affected projects for a target.
With the run-many command filtering is only possible with --projects.
Both accept a --all, it being warned for in affected.

Expected Behavior

We would expect to be able to select projects by tags.

This PR

This could be seen as a first step only acting to select projects in the same way --projects does it for run-many. Tags are not applied in compound but rather as OR statement in comma separated fashion.

That is in the example below the command would select projects that either contain the tag sdk or the tag app, not necessarily projects containing both tags.

nx affected --tags=sdk,app --target=test

Related Issue(s)

This relates to very long standing #1621, #2675 or #8292 and a Nx v14 compatible treatment while being different from #8364 which attempts to support globs but does not address run-many.

Closes #1621
Closes #2675

@vercel
Copy link

vercel bot commented May 1, 2022

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

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 6, 2023 6:30pm

@fguitton
Copy link
Contributor Author

fguitton commented May 1, 2022

Hello @vsavkin !
I know you had interest in looking at this over the past few years.
It is the second PR opened for this feature but it addressed things slightly differently.
I am happy to bring any changes required, any feedback/direction is most welcome
We have a lot of need for this in our company.

Thank you for your help and valuable time 🙏🏼 !

@gioragutt
Copy link
Contributor

Any update on this? I would love to use it 😁 Also, I thought it might be worth adding this to print-affected as well

@fguitton
Copy link
Contributor Author

I thought it might be worth adding this to print-affected as well

Hello ! The print-affected command should already work out of the box with the changes proposed at it is considered a special branch of the affected command and the project filtering gets done upstream of that.

const projects = projectsToRun(nxArgs, projectGraph);

case 'print-affected':
if (nxArgs.target) {
const projectsWithTarget = allProjectsWithTarget(projects, nxArgs);
await printAffected(
projectsWithTarget,
projects,
projectGraph,
env,
nxArgs,
overrides
);
} else {
await printAffected(
[],
projects,
projectGraph,
env,
nxArgs,
overrides
);
}
break;
case 'affected': {
const projectsWithTarget = allProjectsWithTarget(projects, nxArgs);
await runCommand(
projectsWithTarget,
projectGraph,
env,
nxArgs,
overrides,
null
);
break;
}

I can imagine there is latent question with regard to the use/semantic of --exclude which only applies to project names. I do not necessarily have a set opinion on this. In effect a tag filtering can very much replace the use of --exclude.

@fguitton fguitton changed the title feat(core): add support for tags with affected and run-many feat(core): add support for tags with (print-)affected and run-many May 17, 2022
@fguitton fguitton changed the title feat(core): add support for tags with (print-)affected and run-many feat(core): add support for tags with (print-)affected(:*) and run-many May 17, 2022
@fguitton fguitton force-pushed the feat/affected-tags-filter branch from 9dafbd4 to 09a5683 Compare May 17, 2022 09:14
@yharaskrik
Copy link
Contributor

Would love to see this merged into Nx and my PR closed.

@epechuzal
Copy link

epechuzal commented Jun 24, 2022

This would be a big QoL feature for us, would love to see this in the next release! Any indications when that might be?

packages/nx/src/utils/find-matching-projects.ts Outdated Show resolved Hide resolved
if (minimatch.match(tags, pattern.value).length)
(pattern.exclude ? excludedProjects : selectedProjects).add(
projectName
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

future: I think we should consider caching the tags which match a pattern.

Because projects usually share the same tag, there's no point in matching the pattern with the same tag over and over.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'll look at this in the future

packages/nx/src/command-line/examples.ts Outdated Show resolved Hide resolved
@AgentEnder AgentEnder force-pushed the feat/affected-tags-filter branch from b9fa86a to ca6d497 Compare April 6, 2023 15:20
@AgentEnder AgentEnder merged commit 454344b into nrwl:master Apr 6, 2023
@fguitton
Copy link
Contributor Author

fguitton commented Apr 6, 2023

Thank you @AgentEnder ! Extending to the whole Nx team of course ! Thrilled to see this merged 🎉

@fguitton fguitton deleted the feat/affected-tags-filter branch April 6, 2023 18:47
@gioragutt
Copy link
Contributor

Congrats @fguitton, you've put a lot of work on this and it shows! Thank you for all your effort, I'm sure it'll bring immense value to a lot of people 🙏🏻🙏🏻🙏🏻

@github-actions
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 Apr 12, 2023
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.

[FEAT] run-many --tags=tag1,tag2 Exclude Apps in affected scripts by tags from nx.json