Skip to content

Conversation

feniljain
Copy link
Contributor

@feniljain feniljain commented Oct 6, 2022

Should fix #13312

Didn't add a test because I was not sure on how to add test for a specific configuration option, tried to look for the usages for other AssistConfig variants but couldn't find any in tests. If there is a way to test this, do point me towards it.

I tried to extract the formatting string as a common template_string and only have if-else for that, but it didn't compile :(

Also it seems these tests are failing:

test config::tests::generate_config_documentation ... FAILED
test config::tests::generate_package_json_config ... FAILED

Can you also point me to how to correct these 😅 ( I guess there is some command to automatically generate these? )

@Veykril
Copy link
Member

Veykril commented Oct 10, 2022

cargo test -p rust-analyzer should regenerate the files for the failing tests (the test itself will fail once as it updates, the next run should then succeed)

@feniljain feniljain force-pushed the feat-must-use-option branch from d46acc9 to 4b44a1d Compare October 10, 2022 13:19
@bors
Copy link
Contributor

bors commented Oct 17, 2022

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

@feniljain feniljain force-pushed the feat-must-use-option branch 3 times, most recently from eeb20d3 to f32e9b6 Compare October 19, 2022 07:55
@feniljain feniljain force-pushed the feat-must-use-option branch from f32e9b6 to c4bdb8e Compare October 24, 2022 07:48
@feniljain feniljain force-pushed the feat-must-use-option branch from 6daf0d9 to c2bdde9 Compare October 24, 2022 15:40
@feniljain feniljain force-pushed the feat-must-use-option branch from c2bdde9 to 4bf9b9b Compare October 24, 2022 15:42
@feniljain
Copy link
Contributor Author

Weird, ubuntu one passed, but windows one failed

@Veykril
Copy link
Member

Veykril commented Nov 2, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Nov 2, 2022

📌 Commit 691ce30 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 2, 2022

⌛ Testing commit 691ce30 with merge af1f48d...

@bors
Copy link
Contributor

bors commented Nov 2, 2022

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

@bors bors merged commit af1f48d into rust-lang:master Nov 2, 2022
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.

Add #[must_use] to generated as_ method.
3 participants