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

feat(builder): Updated min_value and required docs #3728

Merged
merged 1 commit into from
May 16, 2022

Conversation

AndreasBackx
Copy link
Contributor

Fixes #3259.

@AndreasBackx AndreasBackx force-pushed the issue/3259 branch 2 times, most recently from de3faaf to 4da709b Compare May 14, 2022 18:07
@AndreasBackx AndreasBackx changed the title feat(builder): min_value and required docs updated. feat(builder): Updated min_value and required docs May 14, 2022
src/builder/arg.rs Outdated Show resolved Hide resolved
src/builder/arg.rs Outdated Show resolved Hide resolved
@AndreasBackx
Copy link
Contributor Author

I'm unsure if force pushed amends or new commits are preferred based on the contributing guidelines as the commits needs to stay atomic and I'm unsure if you will squash or not. Let me know what you prefer, I've been working less on GitHub and more on Phabricator with stacked diffs where this isn't an issue. 😅

Let me know what you think of the changes.

@epage
Copy link
Member

epage commented May 15, 2022

I'm unsure if force pushed amends or new commits are preferred based on the contributing guidelines as the commits needs to stay atomic and I'm unsure if you will squash or not.

Go ahead an force-push

Really wish a tool would support squashing fixup commits so people could just push those for easier reviewing.

Let me know what you prefer, I've been working less on GitHub and more on Phabricator with stacked diffs where this isn't an issue. 😅

Huh, for some reason I thought you guys had moved off Phabricator. Maybe its just different parts? Has the effort to keep it going after the company shut down worked out? At my last place, I saw the writing on the wall before the shutdown and was starting to organize the transition off.

@AndreasBackx
Copy link
Contributor Author

Really wish a tool would support squashing fixup commits so people could just push those for easier reviewing.

I thought that if you squashed on GitHub, you can pick the new message thus I could push fixup commits?

Huh, for some reason I thought you guys had moved off Phabricator. Maybe its just different parts? Has the effort to keep it going after the company shut down worked out? At my last place, I saw the writing on the wall before the shutdown and was starting to organize the transition off.

It was forked a long time ago from what I understand. I think Phabricator works out great internally, I would definitely say it's a better way of working than GitHub/GitLab's pull/merge requests.

@epage
Copy link
Member

epage commented May 16, 2022

I thought that if you squashed on GitHub, you can pick the new message thus I could push fixup commits?

Squashing is brute force; you squash down to one commit when there are valid reasons to preserve multiple commits in a PR.

It was forked a long time ago from what I understand. I think Phabricator works out great internally, I would definitely say it's a better way of working than GitHub/GitLab's pull/merge requests.

I'll assume this is because of local modifications :)

btw I made git stack to get some of the benefits of arc while improving on it in other ways (e.g. better rebasing to preserve the tree of commits/branches).

@epage epage merged commit 740bb39 into clap-rs:master May 16, 2022
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.

min_values when using Parser not respected
2 participants