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

Move getopts out of extra #12007

Merged
merged 4 commits into from
Feb 6, 2014
Merged

Move getopts out of extra #12007

merged 4 commits into from
Feb 6, 2014

Conversation

Arcterus
Copy link
Contributor

@Arcterus Arcterus commented Feb 3, 2014

Should help towards finishing #8784.

@huonw
Copy link
Member

huonw commented Feb 3, 2014

cc @cmr

@emberian
Copy link
Member

emberian commented Feb 3, 2014

Awesome! Needs a rebase, Also, please remove getopts and make getopts::groups the only option.

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 3, 2014

Do you mean replace the base getopts and its functions (e.g. optopt) with the ones in getopts::groups?

@emberian
Copy link
Member

emberian commented Feb 3, 2014

yes

On Mon, Feb 3, 2014 at 12:42 AM, Arcterus notifications@github.com wrote:

Do you mean replace the base getopts and its functions (e.g. optopt) with
the ones in getopts::groups?


Reply to this email directly or view it on GitHubhttps://github.com//pull/12007#issuecomment-33926031
.

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 3, 2014

Alright, working on that. :)

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 4, 2014

@cmr: I believe that I've implemented your request.

@huonw
Copy link
Member

huonw commented Feb 4, 2014

From my brief glance (not a full review), looks good!

However, it looks like there's a few duplicated tests now, and you can probably delete a bunch of them. :)

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 4, 2014

They seem to be mostly the same, but some check for the long form while others check for the short form. Would it be better to keep them separated or would you rather me merge them?

@huonw
Copy link
Member

huonw commented Feb 4, 2014

Merging would be good, otherwise the module will be "infected" by its history (e.g. tests called test_groups_getopts when there is no groups anywhere else).

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 4, 2014

True. I'll try to do that soon.

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 5, 2014

The tests have been merged.

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 5, 2014

@huonw @cmr have either of you had time to review this?

@flaper87
Copy link
Contributor

flaper87 commented Feb 5, 2014

I think this needs to be rebased

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 6, 2014

I have a rebased copy, but I'm waiting for #12001 and #12010 to merge (bors is currently building them).

@Arcterus
Copy link
Contributor Author

Arcterus commented Feb 6, 2014

This is ready to be merged again.

bors added a commit that referenced this pull request Feb 6, 2014
@bors bors closed this Feb 6, 2014
@bors bors merged commit 968ce53 into rust-lang:master Feb 6, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
…, r=jonas-schievink

Restart proc-macro client when server reload

Fix rust-lang#10719
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 11, 2024
…s-with-brackets, r=Jarcho

New Lint: empty_enum_variants_with_brackets

This PR:
- adds a new early pass lint that checks for enum variants with no fields that were defined using brackets. **Category: Restriction**
- adds relevant UI tests for the new lint.

Closes rust-lang#12007

```
changelog: New lint: [`empty_enum_variants_with_brackets`]
```
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.

5 participants