feat(build-cli): Add exec command #14635
Conversation
|
Making the new options work for everything (what you did) seems like the best choice. I think it would be fine to normalize type test information across all release packages after a tooling update for example. |
That's a great scenario that I had not considered. Cool! |
| "Run on the package in this directory. Cannot be used with --releaseGroup or --packages.", | ||
| exclusive: ["packages", "releaseGroup"], | ||
| "Run on the package in this directory. Cannot be used with --all, --packages, or --releaseGroup.", | ||
| exclusive: ["packages", "releaseGroup", "all"], | ||
| }), | ||
| packages: Flags.boolean({ |
There was a problem hiding this comment.
We should probably leave it for a separate PR if we change it, but I find "packages" a confusing option. Renaming this to "independent" or "separate" or "non-grouped" seems like it would be better.
There was a problem hiding this comment.
Good point. I'll do that in a separate PR.
| /** | ||
| * Package is part of a release group | ||
| */ | ||
| | "releaseGroupPackage" |
There was a problem hiding this comment.
This means release group, but not the root of it right? Someone could think that this refers to the package for a release group, which is the root. I think "releaseGroupChild" would be more clear.
Also since all of these are about packages, we should probably make all their names include "package" or none of them, for consistency.
| * Package is being loaded from a directory. The package may be one of the other three kinds. This kind is only used | ||
| * when running on a package diurectly using its directory. | ||
| */ | ||
| | "packageDir"; |
There was a problem hiding this comment.
avoiding abbreviations is preferable for readability and reduced ambiguity. calling this "fromPath" or "directory" might be more clear.
| /** | ||
| * Runs processPackage for each package in a release group, exlcuding the root. | ||
| */ | ||
| private async processReleaseGroup( |
There was a problem hiding this comment.
I think separating the concerns of finding the right packages, and running an operation on them would be cleaner. Having this return a list of packages, then having the user of this add it to the full set its going to process would accomplish this.
This approach would also make actions like detecting duplicates, or applying multiple different operations to a set of packages easier.
It would also allow parallelizing across multiple release groups while having a single scheduler to avoid having too many in progress tasks and mixed status reporting.
There was a problem hiding this comment.
Also making async methods have useful return values (like the list of packahes) instead of just having sideeffects tends to help reduce the risk or unawaited promises or unexpected concurrency.
| /** | ||
| * Runs processPackage for each independent package in the repo. | ||
| */ | ||
| private async processIndependentPackages(): Promise<void> { |
There was a problem hiding this comment.
Same as above: I think having this return the list of found packages would be better.
| return [{ directory: rg.repoPath, kind: "releaseGroupRootPackage" }]; | ||
| } | ||
|
|
||
| return ctx.packagesInReleaseGroup(releaseGroup).map((p) => { |
There was a problem hiding this comment.
Again, for another PR, but I think a decent amount of logic over in monoRepo.ts could be replaced by a call to releaseGroups to find these packages.
This PR adds an
execcommand to build-cli, which can be used to execute shell commands in the context of packages or release groups in the repo.The following example runs
pnpm -r remove @rushstack/eslint-configon all independent packages and all release group root packages:I think that for this command to be useful, one needs to be able to scope the packages that it runs on to the following groups:
--packagesflag.--scopeand--skipScopeflags.--dirflag.--releaseGroupflag.--releaseGroupand--releaseGroupRootsflags together.--alland--releaseGroupRootsflags together.--allflag.I implemented this by adding two new flags to the BasePackageCommand class,
--alland--releaseGroupRoots. Most of the code change was in the base class. This means that thetypetestcommand can also benefit from the new flags. That said, releaseGroupRoots in particular doesn't make sense for typetests, so perhaps the new flags shouldn't be on the base class? Feedback welcome.