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

Conversation

@h-michael
Copy link
Contributor

related with #1266

Copy link
Contributor

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for writing this up!

I suggested some changes in hope of clarifying it a bit - hope you don't mind =)

contributing.md Outdated
cargo build --release
```
#### If RLS couldn't be built ba clippy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### If RLS couldn't be built ba clippy
#### If RLS couldn't be built with clippy

contributing.md Outdated
#### If RLS couldn't be built ba clippy
Sometimes nightly toolchain changes break `clippy_lints`.(`clippy_lints` is dependent on rust.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Sometimes nightly toolchain changes break `clippy_lints`.(`clippy_lints` is dependent on rust.)
Sometimes nightly toolchain changes break the `clippy_lints` dependency.

I think this is repeating the same thing? + rephrased slightly

contributing.md Outdated
#### If RLS couldn't be built ba clippy
Sometimes nightly toolchain changes break `clippy_lints`.(`clippy_lints` is dependent on rust.)
RLS is dependent on `clippy_lints` by default.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd merge these two lines into something like:
Since RLS depends on `clippy_lints` by default, those changes can also break RLS itself.

contributing.md Outdated
Sometimes nightly toolchain changes break `clippy_lints`.(`clippy_lints` is dependent on rust.)
RLS is dependent on `clippy_lints` by default.
So if nightly can't build `clippy_lints`, also nightly can't build RLS.
In this case, you can build RLS like this.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In this case, you can build RLS like this.
In this case, you can build RLS like this:

contributing.md Outdated
`cargo build --no-default-features` (disabling the clippy feature)
And sometimes git revision of `clippy_lints` of rust (https://github.com/rust-lang/rust/tree/master/src/tools) and `clippy_lints` of RLS is different.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
And sometimes git revision of `clippy_lints` of rust (https://github.com/rust-lang/rust/tree/master/src/tools) and `clippy_lints` of RLS is different.
And sometimes git revision of `clippy` submodule in the Rust repo (https://github.com/rust-lang/rust/tree/master/src/tools) and `clippy_lints` dependency of RLS is different.

git clone https://github.com/rust-lang/rls.git
cd rls
cargo build --release
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's deleted by mistake

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's 6 backticks instead of none 😅
No worries, I'll fix that myself - thanks for writing that up!

@h-michael
Copy link
Contributor Author

@Xanewok Thanks for your review and I'm sorry for my poor English.

@h-michael
Copy link
Contributor Author

CI has failed. But that is caused by clippy.

git clone https://github.com/rust-lang/rls.git
cd rls
cargo build --release
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's 6 backticks instead of none 😅
No worries, I'll fix that myself - thanks for writing that up!

@Xanewok Xanewok merged commit bdc3932 into rust-lang:master Jan 28, 2019
@h-michael h-michael deleted the contributingmd branch January 28, 2019 22:42
bors added a commit to rust-lang/rust that referenced this pull request Feb 2, 2019
submodule: update rls from c9d25b to e2145d

Update rls rust-lang/rls@c9d25b6...e2145d

rust-lang/rls#1276 - h-michael:clippy, r=Xanewok
rust-lang/rls#1269 - rust-lang:dependabot/cargo/rand-0.6.5, r=Xanewok
Remove extra backticks in contributing.md
rust-lang/rls#1267 from h-michael/contributingmd
rust-lang/rls#1268 from matthiaskrgr/rustup
rust-lang/rls#1262 from rust-lang/dependabot/cargo/tokio-0.1.15
rust-lang/rls#1264 - h-michael:pub-crate, r=alexheretic
rust-lang/rls#1261 - rust-lang:dependabot/cargo/tokio-timer-0.2.9, r=Xanewok
rust-lang/rls#1263 - Xanewok:update-clippy, r=Xanewok
rust-lang/rls#1257 from Xanewok/architecture
rust-lang/rls#1258 - rust-lang:dependabot/cargo/lsp-types-0.55.1, r=Xanewok
rust-lang/rls#1255 - Xanewok:you-only-complete-once-fool, r=Xanewok
rust-lang/rls#1252 - rust-lang:dependabot/cargo/cargo_metadata-0.7.0, r=alexheretic
rust-lang/rls#1253 - rust-lang:dependabot/cargo/lsp-types-0.55.0, r=Xanewok
rust-lang/rls#1254 - rust-lang:dependabot/cargo/serde_json-1.0.37, r=Xanewok
dependabot: Explicitly list default allowed_updates
dependabot: Add automerge strategy for clippy_lints
rust-lang/rls#1251 - Xanewok:translate-deglob-test, r=Xanewok
rust-lang/rls#1250 from alexheretic/master
rust-lang/rls#1244 - Xanewok:translate-tests, r=alexheretic
rust-lang/rls#1247 - alexheretic:register-more-clippy, r=Xanewok
rust-lang/rls#1230 - emilio:testing-testing, r=Xanewok
rust-lang/rls#1246 from alexheretic/did-save-manifest
Merge branch 'beta-version-bump' of https://github.com/rust-lang-nursery/rls
bors added a commit to rust-lang/rust that referenced this pull request Feb 3, 2019
submodule: update rls from c9d25b to f331ff7

Update rls rust-lang/rls@c9d25b6...e2145d

rust-lang/rls#1276 - h-michael:clippy, r=Xanewok
rust-lang/rls#1269 - rust-lang:dependabot/cargo/rand-0.6.5, r=Xanewok
Remove extra backticks in contributing.md
rust-lang/rls#1267 from h-michael/contributingmd
rust-lang/rls#1268 from matthiaskrgr/rustup
rust-lang/rls#1262 from rust-lang/dependabot/cargo/tokio-0.1.15
rust-lang/rls#1264 - h-michael:pub-crate, r=alexheretic
rust-lang/rls#1261 - rust-lang:dependabot/cargo/tokio-timer-0.2.9, r=Xanewok
rust-lang/rls#1263 - Xanewok:update-clippy, r=Xanewok
rust-lang/rls#1257 from Xanewok/architecture
rust-lang/rls#1258 - rust-lang:dependabot/cargo/lsp-types-0.55.1, r=Xanewok
rust-lang/rls#1255 - Xanewok:you-only-complete-once-fool, r=Xanewok
rust-lang/rls#1252 - rust-lang:dependabot/cargo/cargo_metadata-0.7.0, r=alexheretic
rust-lang/rls#1253 - rust-lang:dependabot/cargo/lsp-types-0.55.0, r=Xanewok
rust-lang/rls#1254 - rust-lang:dependabot/cargo/serde_json-1.0.37, r=Xanewok
dependabot: Explicitly list default allowed_updates
dependabot: Add automerge strategy for clippy_lints
rust-lang/rls#1251 - Xanewok:translate-deglob-test, r=Xanewok
rust-lang/rls#1250 from alexheretic/master
rust-lang/rls#1244 - Xanewok:translate-tests, r=alexheretic
rust-lang/rls#1247 - alexheretic:register-more-clippy, r=Xanewok
rust-lang/rls#1230 - emilio:testing-testing, r=Xanewok
rust-lang/rls#1246 from alexheretic/did-save-manifest
Merge branch 'beta-version-bump' of https://github.com/rust-lang-nursery/rls
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.

2 participants