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

Remove long diagnostic for E0002 #37058

Closed
wants to merge 1 commit into from

Conversation

jfirebaugh
Copy link
Contributor

E0002 was merged into E0004 in #36909, and the long diagnostic for E0004 is sufficient to cover empty match expressions.

cc @GuillaumeGomez

r? @jonathandturner

E0002 was merged into E0004 in rust-lang#36909, and the long diagnostic for E0004 is sufficient to cover empty match expressions.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jonathandturner (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@sophiajt
Copy link
Contributor

sophiajt commented Oct 9, 2016

Good catch

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 9, 2016

📌 Commit c4abe9d has been approved by jonathandturner

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Oct 10, 2016

Sorry, I have memory of an internal policy which wants to keep old error codes explanation (even unused ones). Maybe I'm wrong. If it's the case, I'll r+ this PR again as soon as I have confirmation.

@bors: r-

@sophiajt
Copy link
Contributor

Pinging @brson to see if he can confirm the internal policy around keeping error code long explanations for errors that no longer exist

@GuillaumeGomez
Copy link
Member

cc @brson

@bors
Copy link
Contributor

bors commented Oct 27, 2016

☔ The latest upstream changes (presumably #36695) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to resubmit with a rebase!

@jfirebaugh
Copy link
Contributor Author

@alexcrichton If I rebase, will it actually move this PR along? AFAICT, it's waiting on comment from Powers That Be, not me.

@brson brson reopened this Nov 10, 2016
@brson
Copy link
Contributor

brson commented Nov 10, 2016

@GuillaumeGomez I'm not sure what current practice is, but keeping old error explanations around sounds like something I might advocate, since there may be existing URLS in the wild. Certainly the old codes must remain allocated and unused, if not documented.

I don't feel strongly either way, whether old descriptions are kept or removed, but it seems like we should be consistent. Perhaps you can just decide.

@jfirebaugh Sorry for the delay.

@brson
Copy link
Contributor

brson commented Nov 10, 2016

It looks to me like previously we've just added a notice to the top of such errors: https://github.com/rust-lang/rust/blob/master/src/librustc_resolve/diagnostics.rs#L19.

So I think we should just add the line

## Note: this error code is no longer emitted by the compiler.

as the first line of this error code.

@jfirebaugh I understand if you aren't interested in making the change. If you are then please go ahead, otherwise I'll follow up later.

@alexcrichton
Copy link
Member

@jfirebaugh ah sorry I should have read more closely! I was just some standard triage and clearing out the queue a bit. I've personally seen a good track record for resubmitted rather than revived PRs in the past, but I always love to be proven wrong!

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 11, 2016

Note: this error code is no longer emitted by the compiler.

@brson: I think this is a good compromise.
@jfirebaugh: Can you add the line at the beginning of the error explanation instead of removing it please?

@jfirebaugh
Copy link
Contributor Author

Thanks for the help -- I'm going to have to move on to other priorities however.

@jfirebaugh jfirebaugh closed this Nov 13, 2016
@jfirebaugh jfirebaugh deleted the E00002 branch November 13, 2016 22:04
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 17, 2016
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 17, 2016
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.

7 participants