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

Add --keep-going to the check command #17561

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Conversation

mo8it
Copy link
Contributor

@mo8it mo8it commented Jul 7, 2024

Fixes rust-lang/rustlings#1628

@Veykril I am not sure about what you meant with "unconditionally" in rust-lang/rustlings#1628 (comment), but I didn't find out how to get the version of the toolchain anyway to do a check like in this snippet. Is this check even required if rust-analyzer was installed with the toolchain?

--keep-going was stabilized in 1.74

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2024
@Veykril
Copy link
Member

Veykril commented Jul 8, 2024

The check isn't require as we don't support toolchains that old anyways (it is impossible to reliably support a big range of rust versions)
Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Jul 8, 2024

📌 Commit 9d01d7c has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 8, 2024

⌛ Testing commit 9d01d7c with merge 8f841ca...

@bors
Copy link
Collaborator

bors commented Jul 8, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 8f841ca to master...

@bors bors merged commit 8f841ca into rust-lang:master Jul 8, 2024
11 checks passed
@mo8it mo8it deleted the keep-going branch July 8, 2024 09:15
@mo8it
Copy link
Contributor Author

mo8it commented Jul 8, 2024

@Veykril Should I remove the check here then?

@lnicola
Copy link
Member

lnicola commented Jul 8, 2024

Yeah, we can probably drop it, it's been a while.

bors added a commit that referenced this pull request Jul 8, 2024
Remove version check before using `--keep-going`

See #17561 (comment) by `@lnicola`
lnicola pushed a commit to lnicola/rust that referenced this pull request Jul 11, 2024
Remove version check before using `--keep-going`

See rust-lang/rust-analyzer#17561 (comment) by `@lnicola`
@huntc
Copy link

huntc commented Jul 22, 2024

The check isn't require as we don't support toolchains that old anyways (it is impossible to reliably support a big range of rust versions) Thanks! @bors r+

This has caused a problem with a couple of my projects that are pinned to 1.72 of Rust. Is there a way we can make this conditional e.g. don't include the check if the compiler is pre-1.74?

Please see #17662.

I also understand that there's no strict backward-compability statement for RA, but Zed and VSC don't appear to honour rust-toolchain rust-analyzer components, so we're a bit stuck. Also see zed-industries/zed#4883 (comment)

@devanlai
Copy link

The check isn't require as we don't support toolchains that old anyways (it is impossible to reliably support a big range of rust versions) Thanks! @bors r+

This has caused a problem with a couple of my projects that are pinned to 1.72 of Rust. Is there a way we can make this conditional e.g. don't include the check if the compiler is pre-1.74?

Please see #17662.

I also understand that there's no strict backward-compability statement for RA, but Zed and VSC don't appear to honour rust-toolchain rust-analyzer components, so we're a bit stuck. Also see zed-industries/zed#4883 (comment)

I ran into the same issue with being pinned to an older rust version. For VSCode at least, I've found that using rust-analyzer's config to use the stable toolchain instead seems to work fine, presuming that for analysis purposes, the stable toolchain is fine.

@Veykril
Copy link
Member

Veykril commented Jul 22, 2024

but Zed and VSC don't appear to honour rust-toolchain rust-analyzer components, so we're a bit stuck.

That is a good point to raise, we should be able to support the toolchain component in an overwrite if its installed imo. Though that is the job of the client as that is the entity responsible for spawning the process. We can do this in VSCode but Zed would need to do that themselves.

@Veykril
Copy link
Member

Veykril commented Jul 22, 2024

I'll try to look into the VSCode thing here #17663, zed can then probably try to do something similar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some compiler errors are not highlighted in editor (vscode)
7 participants