Skip to content

Link to Git guide in rustc-dev-guide on merge conflicts #103

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

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

camelid
Copy link
Member

@camelid camelid commented Sep 29, 2020

Dependent on rust-lang/rustc-dev-guide#890 being merged, so this is draft.

That PR was merged, so this is ready!

@camelid camelid marked this pull request as ready for review September 30, 2020 16:20
@jyn514
Copy link
Member

jyn514 commented Oct 6, 2020

@Mark-Simulacrum can we get this merged? I think it would be really helpful.

@Mark-Simulacrum
Copy link
Member

Why are there noqa trailers? Do they mean something?

@jyn514
Copy link
Member

jyn514 commented Oct 6, 2020

@camelid
Copy link
Member Author

camelid commented Oct 6, 2020

I didn’t understand them either when I first looked at this code :)

CI fails if you have a line longer than 79 chars without “noqa”, and it’s hard to keep the lines short for these messages. (“noqa” tells the linter to ignore the line.) I assume “noqa” stands for “no quality assurance” or something.

@Mark-Simulacrum
Copy link
Member

I think fixing CI shouldn't be too hard, could you try to do that? It feels odd to only do it here.

@camelid
Copy link
Member Author

camelid commented Oct 6, 2020

This spot had noqa before I worked on, but I agree that it’s kind of annoying. By “fix CI” do you mean increase the line length limit or something else?

@Mark-Simulacrum
Copy link
Member

I don't see noqa in the lines removed? Anyway, not worth fighting about, let's just merge it in.

@Mark-Simulacrum Mark-Simulacrum merged commit bd165b4 into rust-lang:master Oct 6, 2020
@camelid camelid deleted the link-to-git-guide branch October 6, 2020 17:09
@camelid
Copy link
Member Author

camelid commented Oct 6, 2020

I changed the code a few weeks ago and was able to remove noqa (the line was 130 chars or something like that before). But the link alone is longer than the limit here, and it doesn’t seem worth splitting up to satisfy the linter since I think it’s confusing to see it on multiple lines.

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