-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Non-breaking change* from UK spellings to US #8291
Conversation
# Objective In the [`Text`](https://github.com/bevyengine/bevy/blob/3442a13d2cb4b2d77c9f7bf8b6dad25742bd21d7/crates/bevy_text/src/text.rs#L18) struct the field is named: `linebreak_behaviour`, the British spelling of _behavior_. **Update**, also found: - `FileDragAndDrop::HoveredFileCancelled` - `TouchPhase::Cancelled` - `Touches.just_cancelled` The majority of all spelling is in the US but when you have a lot of contributors across the world, sometimes spelling differences can pop up in APIs such as in this case. For consistency, I think it would be worth a while to ensure that the API is persistent. Some examples: `from_reflect.rs` has `DefaultBehavior` TextStyle has `color` and uses the `Color` struct. In `bevy_input/src/Touch.rs` `TouchPhase::Cancelled` and _canceled_ are used interchangeably in the documentation I've found that there is also the same type of discrepancies in the documentation, though this is a low priority but is worth checking. **Update**: I've now checked the documentation (See #8291) ## Solution I've only renamed the inconsistencies that have breaking changes and documentation pertaining to them. The rest of the documentation will be changed via #8291. Do note that the winit API is written with UK spelling, thus this may be a cause for confusion: `winit::event::TouchPhase::Cancelled => TouchPhase::Canceled` `winit::event::WindowEvent::HoveredFileCancelled` -> Related to `FileDragAndDrop::HoveredFileCanceled` But I'm hoping to maybe outline other spelling inconsistencies in the API, and maybe an addition to the contribution guide. --- ## Changelog - `Text` field `linebreak_behaviour` has been renamed to `linebreak_behavior`. - Event `FileDragAndDrop::HoveredFileCancelled` has been renamed to `HoveredFileCanceled` - Function `Touches.just_cancelled` has been renamed to `Touches.just_canceled` - Event `TouchPhase::Cancelled` has been renamed to `TouchPhase::Canceled` ## Migration Guide Update where `linebreak_behaviour` is used to `linebreak_behavior` Updated the event `FileDragAndDrop::HoveredFileCancelled` where used to `HoveredFileCanceled` Update `Touches.just_cancelled` where used as `Touches.just_canceled` The event `TouchPhase::Cancelled` is now called `TouchPhase::Canceled`
Marking this as controversial due to updating project guidelines. To make sure this is a direction we want to go in. |
I'm opposed to enforcing this without an automated way to check this. |
@alice-i-cecile Working on it 🚀 #https://github.com/BlackPhlox/bevy/pull/1 |
If we can get this in CI without massive false positives I'm on board with it for consistency :) Ideally we're also checking type and method names too? I think that rather than using a generic spellchecker, we should probably consider a blacklist approach? Software dev always has a ton of "unusual" words, and game dev is even worse for that. |
@alice-i-cecile Agreed! I think it might create too much friction for everyone involved if it is a whitelist. So I think we should wait until blacklist functionality has been developed for check-spelling, see check-spelling - Feature : Dictionary deltas. So currently I'm just testing by compiling my own whitelist just to see if it would work from a CI standpoint. See latest CI run for how it looks currently. (Note: the gh action does have an accompanying bot) |
@alice-i-cecile I've updated my opinion on this and reflected my concerns in #8324. I think we should continue the discussion to that pr instead. Maybe then I should remove engine_style_guide.md changes from this pr, so this pr is trivial instead of marked controversial and do the change in #8324 instead? |
Agreed on moving that, and removing the style guide changes :) |
Canadian spelling are obviously the superior option, but I definitely think we should standardize, even if informally. Merging now; we can continue the discussion on automated tools to enforce this. |
Head branch was pushed to by a user without write access
I don't agree that style guide changes need to be accompanied by automated enforcement. They're just guidelines, and I think that suggesting a standard style is better than being silent on it. Plus, international spellings will most likely slip past code review in the future, so having this in the style guide would make it easier for contributors to write a PR to fix them (gives them an easy motivation to refer to, which can be the scariest part of making a PR for first-time contributors). That said, the style guide changes should probably be in a separate PR. |
Fixes issue mentioned in PR #8285.
Note: By mistake, this is currently dependent on #8285
Objective
Ensure consistency in the spelling of the documentation.
Exceptions:
crates/bevy_mikktspace/src/generated.rs
- Has not been changed from licence to license as it is part of a licensing agreement.Maybe for further consistency, https://github.com/bevyengine/bevy-website should also be given a look.
Solution
Changed the spelling of the current words (UK/CN/AU -> US) :
cancelled -> canceled (Breaking API changes in #8285)
behaviour -> behavior (Breaking API changes in #8285)
neighbour -> neighbor
grey -> gray
recognise -> recognize
centre -> center
metres -> meters
colour -> color
Update [Moved to #8324engine_style_guide.md
]Changelog
Changed UK spellings in documentation to US
Migration Guide
Non-breaking changes*
* If merged after #8285