Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Remove license headers #1235

Merged
merged 2 commits into from
Jan 12, 2019
Merged

Remove license headers #1235

merged 2 commits into from
Jan 12, 2019

Conversation

jens1o
Copy link
Contributor

@jens1o jens1o commented Jan 11, 2019

I noticed I forgot to push to a different branch, I'm sorry.

Furthermore, I needed to revert some rustfmt actions, where I was unsure whether they are wanted(the json formatting seems okay to me, but I can delete this on request).

fixes #1231

@jens1o jens1o changed the title Fix https://github.com/rust-lang/rls/issues/1231 Fix #1231 Jan 11, 2019
@Xanewok
Copy link
Member

Xanewok commented Jan 11, 2019

Thank you, it's a great first PR!

Since in our tests/* we're verifying if the results are on a correct line and this removes the first couple of lines of copyright notice, the line numbers in tests need to be adjusted to match the changed files (also json formatting change is fine, don't worry about it).

@jens1o
Copy link
Contributor Author

jens1o commented Jan 11, 2019

Thanks for your quick and detailed response. :) I missed that(because I didn't knew it is verifying this by static results). I'll fix that tomorrow. :) Too tired..

@Xanewok
Copy link
Member

Xanewok commented Jan 11, 2019

No worries, thanks!

@jens1o
Copy link
Contributor Author

jens1o commented Jan 12, 2019

I think that should be it, but maybe I missed some tests(because I've got multiple ICEs while testing) although I've got test result: ok. 29 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out in the end overall for cargo test --test tests_old.

@Xanewok
Copy link
Member

Xanewok commented Jan 12, 2019

Yeah, that's good. The ICE fix is pending at rust-lang/rust#57436, no need to worry about that.

Just a heads up - it's good to include in the PR title what's the actual change (e.g. "Remove copyright headers") instead of the issue number. This way everyone can know at a glance what the PR is about and you're probably going to write

fixes #issue_no

in the description anyway, so GitHub (and others!) knows what's the related issue.

But that's super minor; thank you for your work!

@Xanewok Xanewok merged commit 7ae6006 into rust-lang:master Jan 12, 2019
@jens1o jens1o changed the title Fix #1231 Remove license headers Jan 12, 2019
@jens1o
Copy link
Contributor Author

jens1o commented Jan 12, 2019

Thanks for the heads up. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove license headers
2 participants