-
Notifications
You must be signed in to change notification settings - Fork 38
Allow setting --max-workers / other publish options #32
Comments
Update: Yep! Can't share my actual build log as it's a private project, but https://github.com/motiz88/appr/tree/limit-max-workers did the trick. I have seen Travis occasionally barf on builds that spawned "too many" processes (Jest used to suffer from this I think), so I guess this is one of those cases. Also, I'm pretty sure this isn't a false positive; my build failures without this patch have been very consistent, across CI runs, across versions of So, now that it's fully justified - back to the feature discussion 😄 Happy to put together a PR with whatever API is agreed here. |
@motiz88 this is good. API-wise, I think we should just whitelist any args that Appr needs itself (currently none) and pass forward any remaining Alternatively we can whitelist all the known What's your preference @motiz88? |
One subtle issue with a generic solution is that |
Right you are. Let me noodle about this a little bit. If you have a concrete suggestion I'm happy to hear it. Some ideas:
|
I like Maybe Thing is, all the generic solutions seem to have edge cases that probably exceed the complexity budget here. I'm kinda leaning towards whitelisting options one-by-one (either |
Background
I'm encountering a similar issue to #21 (only it's on Travis rather than CircleCI).
ssh
ing into the build worker and tweakingappr/index.js
, I am able to get it to publish usingexp publish --max-workers=1
(introduced inexp
v50.0.0). (I'm going to try using a quick fork ofappr
to validate that this completely solves the issue, will report back)Feature suggestions
What do you think of having a generic way to pass options to
exp publish
throughappr
? Or at least a way to pass--max-workers
through? Or even just defaulting to--max-workers=1
?The text was updated successfully, but these errors were encountered: