Skip to content
This repository has been archived by the owner on Mar 4, 2022. It is now read-only.

Chore: Add --retries=NUM to builder run #38

Merged
merged 2 commits into from
Dec 2, 2015
Merged

Conversation

ryan-roemer
Copy link
Member

This PR:

Fixes #16
Fixes #37

My use case for this is retries for requirepack functional tests (where I can't use magellan).

/cc @chaseadamsio @boygirl @exogen

Fix args flags object.

Add general help

More help parsing / output.

Refactor flags into first-class object.

Add run --retries option. Also add builder run === builder help. Fixes #37

Add in retries for run.

Remove local npm link stuff.

NIT: remove newline
retries: {
desc: "Number of times to re-run task on non-zero exit (default: `1`)",
types: [Number]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Slightly confusing: the wording makes me think the default is to try twice (1 initial attempt, 1 retry). But looking at the retry code I think the default is 1 attempt and no retries. IMO either this should be named --attempts with a default of 1, or have --retries default to 0 (add 1 to whatever number is given, for the initial attempt, or use doWhilst instead of whilst).

Copy link
Member Author

Choose a reason for hiding this comment

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

@exogen -- How about --tries|attempts=1? or do you think --retries being 1 + NUM_RETRIES is more straightforward?

Copy link
Member Author

Choose a reason for hiding this comment

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

Went with --tries

@exogen
Copy link
Contributor

exogen commented Dec 2, 2015

👍

ryan-roemer added a commit that referenced this pull request Dec 2, 2015
Chore: Add `--retries=NUM` to `builder run`
@ryan-roemer ryan-roemer merged commit db8358e into master Dec 2, 2015
@ryan-roemer ryan-roemer deleted the chore-retries branch December 2, 2015 02:31
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