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 GAT related tests #4970

Merged
merged 4 commits into from
Sep 3, 2021
Merged

Add GAT related tests #4970

merged 4 commits into from
Sep 3, 2021

Conversation

cowboy-bebug
Copy link
Contributor

@cowboy-bebug cowboy-bebug commented Aug 30, 2021

@calebcartwright
Copy link
Member

Thanks! Could you make sure none of those files already exist with _ in the filename instead of - (e.g. issue_4943.rs vs issue-4943.rs)? I feel like there's probably some overlap.

This is the correct behaviour right? Here's another similar example:

Yes indeed, the Rust Style Guide prescribes this formatting for where clauses very explicitly here.

@cowboy-bebug
Copy link
Contributor Author

@calebcartwright Yep there aren't any duplicates. Do you want me to add a commit to mv all issue_XXXX.rs -> issue-XXXX.rs?

@calebcartwright
Copy link
Member

Do you want me to add a commit to mv all issue_XXXX.rs -> issue-XXXX.rs?

Thank you for the offer but no let's not. That's because I actually want our test files to follow the same _ file name convention that we follow for our own source files and which in my experience has been the convention in the other rust-lang/* projects. A change like that would also be really noisy and would be better suited to its own PR.

@calebcartwright Yep there aren't any duplicates.

Oh I see, you've renamed the file for 4943. Please keep the _ as is with existing files, and update the names of the new files added here with _ as well. Apologies, I know we don't have this documented and currently have a mix with a greater portion of - but since we're already on the topic let's stick with the strategic target of _ 😄

@cowboy-bebug
Copy link
Contributor Author

The files are all snake_cased now 😄

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

@calebcartwright calebcartwright merged commit ae5696a into rust-lang:master Sep 3, 2021
@cowboy-bebug cowboy-bebug deleted the cb/4969-add-gat-related-tests branch September 3, 2021 02:01
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 any relevant test cases for reported GAT issues
2 participants