Skip to content

Conversation

@Alexendoo
Copy link
Member

#9605 had me wondering how the edition revision tests were working for manual_assert but not for @nyurik, but it turns out manual_assert's tests weren't working either. I checked how rust-lang/rust does it and apparently it comes down to whitespace, //[rev] edition:X works 😬

Removes the revisions from match_wild_err_arm as I couldn't find any edition dependant behaviour there

r? @llogiq

changelog: none

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 13, 2022
Comment on lines +2 to +3
//[edition2018] edition:2018
//[edition2021] edition:2021
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious: Does this compile the code under both revisions?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does yeah, a test is compiled once per revision, the [revision] syntax makes the magic comment thingies only apply on that revision

@llogiq
Copy link
Contributor

llogiq commented Oct 13, 2022

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 13, 2022

📌 Commit 124caeb has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 13, 2022

⌛ Testing commit 124caeb with merge 4e89ffa...

@bors
Copy link
Contributor

bors commented Oct 13, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 4e89ffa to master...

@bors bors merged commit 4e89ffa into rust-lang:master Oct 13, 2022
@Alexendoo Alexendoo deleted the edition-revisions branch October 13, 2022 12:36
@nyurik
Copy link
Contributor

nyurik commented Oct 13, 2022

Thanks for digging into this! I feel slightly validated that I wasn't going crazy with it :) That said, ideally spacing shouldn't play this much of a role IMO, and perhaps we should get a PR for the upstream to ignore extra spaces? Regardless, thx!

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.

5 participants