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

Migrate framework scene and resources folders to single quotes #4167

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

willeastcott
Copy link
Contributor

This is the second phase of migrating the engine to standardize on single quotes. This time, the folders updated are:

  • framework/components (I left out the files in the root of framework because @mvaligursky has a PR open involving AppBase)
  • resources
  • scene

I suspect we'll need one or possibly two more phases to complete the switch.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@willeastcott willeastcott requested a review from a team April 2, 2022 22:31
@willeastcott willeastcott self-assigned this Apr 2, 2022
@willeastcott willeastcott changed the title Migrate framework scene and resources folders to single quotes Migrate framework scene and resources folders to single quotes Apr 2, 2022
@slimbuck
Copy link
Member

slimbuck commented Apr 4, 2022

I'm wondering how you've make these changes, by hand or find/replace? There are so many changes here I can't imagine actually checking them all!

The only danger I could think of is strings with embedded ' and " characters.

@willeastcott
Copy link
Contributor Author

willeastcott commented Apr 4, 2022

@slimbuck It's 100% performed by doing:

npm run lint -- --fix

So it's reasonable to expect the transformation to be completely trusted.

That said, I did check all the changes visually. 😉 And quotes embedded in strings are happily handled by eslint.

@willeastcott
Copy link
Contributor Author

BTW, I hope that you haven't been fixing lint problems by hand! You just need to run the above command and it's done for you. At least, it is for lint errors that can be auto-fixed.

@slimbuck
Copy link
Member

slimbuck commented Apr 4, 2022

Ahh yes of course I think you mentioned this before. In that case, why not just convert the whole engine?

@willeastcott
Copy link
Contributor Author

Because I'm trying to avoid merge conflicts for people. For example, @mvaligursky is working in graphics and on the Application class, so just being sensitive to that.

@willeastcott willeastcott merged commit 13c52d0 into main Apr 4, 2022
@willeastcott willeastcott deleted the more-single-quotes branch April 4, 2022 13:16
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.

2 participants