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

Fix experimental notice #688

Closed
wants to merge 2 commits into from

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Mar 16, 2017

Follow up to #683.
I'm sorry, I couldn't help it! 👞 ✨ :)

jlebon added 2 commits March 16, 2017 17:24
This is a follow-up to coreos#683. All the experimental commands will be under
`rpm-ostree ex`, so we remove the flag and instead print the notice
before dispatching the subcommand, where we can print the correct name.
Make `rpm-ostree compose bad-command` and `rpm-ostree ex bad-command`
have the same error message formatting.
@cgwalters
Copy link
Member

Ah, yep, makes sense to me.

What I was trying to say before is basically I think we should take into account the benefits of refactoring - we could have the absolute best command line parsing code in the world and our users probably wouldn't notice the difference.

But I definitely agree this patch is better!

@rh-atomic-bot r+ 0864d81

@rh-atomic-bot
Copy link

⌛ Testing commit 0864d81 with merge bde8456...

rh-atomic-bot pushed a commit that referenced this pull request Mar 16, 2017
Make `rpm-ostree compose bad-command` and `rpm-ostree ex bad-command`
have the same error message formatting.

Closes: #688
Approved by: cgwalters
@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing bde8456 to master...

@jlebon
Copy link
Member Author

jlebon commented Mar 16, 2017

No, I totally understand effort vs effect. I guess to me this goes with #405: it's part of making rpm-ostree/$newname look and feel tight. Consistency, good error msgs, etc... It seems trivial, but altogether they're the sort of things by which e.g. less technical people might judge an app. But yeah, I agree we also have to strike a balance.

@jlebon jlebon deleted the pr/i-just-love-polishing-things branch March 20, 2017 20:42
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.

3 participants