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

(BKR-1168) Exit early on --version, --help, and --parse-only #1523

Merged
merged 2 commits into from
Jun 22, 2018

Conversation

Dakta
Copy link
Contributor

@Dakta Dakta commented Jun 12, 2018

Previously, some code paths could allow continued execution of Beaker after encountering these flags. This resulted in a not-so-dry run scenario which, at best, caused confusing error output when beaker --version was called and, at worst, may have led to the unintentional execution of a configured provisioning task.

@Dakta
Copy link
Contributor Author

Dakta commented Jun 12, 2018

Those failures are the continued Windows cURL issues. Everything else is good.

@kevpl
Copy link
Contributor

kevpl commented Jun 13, 2018

Jenkins, test this please

1 similar comment
@kevpl
Copy link
Contributor

kevpl commented Jun 15, 2018

Jenkins, test this please

@Dakta
Copy link
Contributor Author

Dakta commented Jun 18, 2018

Pending removal of further early-exit conditions based on options[:help].

@Dakta
Copy link
Contributor Author

Dakta commented Jun 18, 2018

Jenkins, retest this please.

Previous "dry-run"-esque behavior was leaky, resulting in a not-so-dry run.

- Subcommand spec tests for exec should not actually call cli.execute!, so intercept them.
- Process all early-exit flags first
@Dakta
Copy link
Contributor Author

Dakta commented Jun 20, 2018

Jenkins, retest this please.

@Dakta
Copy link
Contributor Author

Dakta commented Jun 20, 2018

@kevpl Tests are all pass except for a transient package error on Solaris.

kevpl
kevpl previously requested changes Jun 20, 2018
Copy link
Contributor

@kevpl kevpl left a comment

Choose a reason for hiding this comment

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

lgtm 👍

CHANGELOG.md Outdated
@@ -19,6 +19,7 @@ git logs & PR history.

- Raise `ArgumentError` when passing `role = nil` to `only_host_with_role()` or `find_at_most_one_host_with_role()`
- Use `install_package_with_rpm` in `add_el_extras`
- Exit early on --help/--version/--parse-only arguments instead of partial dry-run
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this line has been pushed down into 3.36.0 with the new release, so this will have to be put back up into the Unreleased section

@kevpl
Copy link
Contributor

kevpl commented Jun 21, 2018

looks like a vmpooler provisioning issue, will rekick.

Jenkins, test this please

Copy link
Contributor

@kevpl kevpl left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@kevpl kevpl merged commit a01b573 into voxpupuli:master Jun 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants