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

✏️ Fix typos across bevy #16702

Merged
merged 1 commit into from
Dec 8, 2024
Merged

Conversation

homersimpsons
Copy link
Contributor

@homersimpsons homersimpsons commented Dec 7, 2024

Objective

Fixes typos in bevy project, following suggestion in bevyengine/bevy-website#1912 (review)

Solution

I used https://github.com/crate-ci/typos to find them.

I included only the ones that feel undebatable too me, but I am not in game engine so maybe some terms are expected.

I left out the following typos:

  • reparametrize => reparameterize: There are a lot of occurences, I believe this was expected
  • semicircles => hemicircles: 2 occurences, may mean something specific in geometry
  • invertation => inversion: may mean something specific
  • unparented => parentless: may mean something specific
  • metalness => metallicity: may mean something specific

Testing

  • Did you test these changes? If so, how? I did not test the changes, most changes are related to raw text. I expect the others to be tested by the CI.
  • Are there any parts that need more testing? I do not think
  • How can other people (reviewers) test your changes? Is there anything specific they need to know? To me there is nothing to test
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?

Migration Guide

This section is optional. If there are no breaking changes, you can delete this section.

(kept in case I include the reparameterize change here)

  • If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes
  • Simply adding new functionality is not a breaking change.
  • Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change.

Questions

This project looks awesome, I really enjoy reading the progress made, thanks to everyone involved.

Copy link
Contributor

github-actions bot commented Dec 7, 2024

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@@ -1384,7 +1384,7 @@ impl AssetServer {
/// asset from being dropped.
/// If you have access to an asset's strong [`Handle`], you should prefer to call
/// [`AssetServer::wait_for_asset`]
/// or [`wait_for_assest_untyped`](Self::wait_for_asset_untyped) to ensure the asset finishes
/// or [`wait_for_asset_untyped`](Self::wait_for_asset_untyped) to ensure the asset finishes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked and the typo was only in the documentation.

@@ -229,7 +229,7 @@ mod test {

let image = Image::from_dynamic(initial.clone(), true, RenderAssetUsages::RENDER_WORLD);

// NOTE: Fails if `is_srbg = false` or the dynamic image is of the type rgb8.
// NOTE: Fails if `is_srgb = false` or the dynamic image is of the type rgb8.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The typo is only in the doc.

@@ -331,7 +331,7 @@ mod tests {
.get::<Parent>()
.expect("something is wrong with this test, and the scene components don't have a parent/child relationship")
.get(),
"something is wrong with the this test or the code reloading scenes since the relationship between scene entities is broken"
"something is wrong with this test or the code reloading scenes since the relationship between scene entities is broken"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not detected by typos but is the original change I wanted to make.

@@ -316,7 +316,7 @@ fn spawn_particle<M: Material>(

#[derive(Component)]
struct Particle {
lifeteime_timer: Timer,
lifetime_timer: Timer,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this change is safe to do, and looks contained to this file. If there should be any debate around it I can revert it.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is fine

@atlv24
Copy link
Contributor

atlv24 commented Dec 7, 2024

i read through these, none of these are controversial thanks

also no breaking changes

@homersimpsons
Copy link
Contributor Author

Thanks @atlv24 for the fast review. Do you have an answer for my 2 questions:

Should I include the above typos?
Should I add typos to the CI? (I will check how to configure it properly)

@BenjaminBrienen
Copy link
Contributor

Don't fix those typos that you correctly left out. Those should be removed from typos-ci.

@mweatherley
Copy link
Contributor

I still don't think 'reparametrize' is a typo or wrong (and I'm sort of the only person using this word either way), insofar as it comes ultimately from the notion of 'parametric' curves. (Perhaps the difference is largely Latin vs. Greek.) As a mathematician, this is how I've always seen this word spelled in this context, and both are regarded as correct.

@rparrett
Copy link
Contributor

rparrett commented Dec 8, 2024

Should I add typos to the CI? (I will check how to configure it properly)

We already use typos in CI, but currently an older version that doesn't catch some of these.

https://github.com/bevyengine/bevy/blob/main/typos.toml
https://github.com/bevyengine/bevy/blob/main/.github/workflows/ci.yml#L239

Related: #16602 (comment)

I agree with the typos you chose not to fix, except perhaps invertation -> inversion seems fine to me.

The other ones should get ignored in the config if we update CI.

@mockersf mockersf added this pull request to the merge queue Dec 8, 2024
Merged via the queue into bevyengine:main with commit 0707c07 Dec 8, 2024
29 checks passed
@homersimpsons homersimpsons deleted the fix/typos branch December 8, 2024 14:01
github-merge-queue bot pushed a commit that referenced this pull request Dec 8, 2024
# Objective

Fixes #16610, related to #16702

## Solution

Upgrade typos and its configuration

## Testing

- Did you test these changes? If so, how? No
- Are there any parts that need more testing? No
- How can other people (reviewers) test your changes? Is there anything
specific they need to know? No
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test? Not applicable
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
# Objective

Fixes typos in bevy project, following suggestion in
bevyengine/bevy-website#1912 (review)

## Solution

I used https://github.com/crate-ci/typos to find them.

I included only the ones that feel undebatable too me, but I am not in
game engine so maybe some terms are expected.

I left out the following typos:
- `reparametrize` => `reparameterize`: There are a lot of occurences, I
believe this was expected
- `semicircles` => `hemicircles`: 2 occurences, may mean something
specific in geometry
- `invertation` => `inversion`: may mean something specific
- `unparented` => `parentless`: may mean something specific
- `metalness` => `metallicity`: may mean something specific

## Testing

- Did you test these changes? If so, how? I did not test the changes,
most changes are related to raw text. I expect the others to be tested
by the CI.
- Are there any parts that need more testing? I do not think
- How can other people (reviewers) test your changes? Is there anything
specific they need to know? To me there is nothing to test
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

(kept in case I include the `reparameterize` change here)

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.

## Questions

- [x] Should I include the above typos? No
(bevyengine#16702 (comment))
- [ ] Should I add `typos` to the CI? (I will check how to configure it
properly)

This project looks awesome, I really enjoy reading the progress made,
thanks to everyone involved.
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
# Objective

Fixes bevyengine#16610, related to bevyengine#16702

## Solution

Upgrade typos and its configuration

## Testing

- Did you test these changes? If so, how? No
- Are there any parts that need more testing? No
- How can other people (reviewers) test your changes? Is there anything
specific they need to know? No
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test? Not applicable
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.

6 participants