-
Notifications
You must be signed in to change notification settings - Fork 394
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
#2026 feat: nest build --all flag #2312
base: master
Are you sure you want to change the base?
Conversation
The implementation has a bug and does not work right now. Can't find a way to put a PR back into draft mode. |
342cab4
to
d441d5c
Compare
The bug should now be fixed. Tested on my local projects, seems good. |
Hi, is there any planned release date for this? We are looking for this functionality. |
Happy New Year everyone~ Is this work in progress? I am waiting for this feature to be released... |
actions/build.action.ts
Outdated
: getBuilder(configuration, commandOptions, appName); | ||
let appNames: string[]; | ||
if (buildAll) { | ||
appNames = [undefined!]; // always include the default project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we change this to be const appNames: string[] | undefined;
the undefined!
looks hacky to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change that, but it requires to also change a lot of other APIs that do accept undefined
to mean "default app", but do not declare that.
I was reluctant to do it, since it's a lot of changes, but if it's okay I'll change those to string | undefined
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this spot and all the APIs it trickles down to in f574cfd.
Haven't looked at other places that accept an appName
- they may also need to be changed to explicitly allow undefined
, but that would be outside of this PR's scope.
179f9c7
to
f574cfd
Compare
I'd like this feature too!! |
any updates on this PR? :) |
please release this feature 🙏🏻 |
Any updates on this? When it about to be released? |
super option! It is very good to be able to generate a single docker image with the entire project and then use it to init 2 or more services that share the same build |
Waiting for this feature as well, I don't want to switch to Nx just because of this |
👀 |
3 similar comments
👀 |
👀 |
👀 |
Hi @kamilmysliwiec, Could you please release this PR? It will greatly help those who have a lot of projects in monorepo. |
f574cfd
to
aef1368
Compare
PR Checklist
Please check if your PR fulfills the following requirements:
(checking the tests box since I don't see a way to add "high-level" tests for an
Action
, only for stuff in/lib
)(docs ==
commander
option descriptions)PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #2026
What is the new behavior?
--all
flag tonest build
to build all projects in a monoreponest build
args (the--all
flag was easier to implement this way :)Does this PR introduce a breaking change?
Other information