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

Global replace [ \t]+$, add "GB" #1751

Merged
merged 5 commits into from
Dec 19, 2022
Merged

Conversation

thinkyhead
Copy link
Contributor

  • Strip all trailing whitespace from text files.
  • Add missing "GB" designation on download size.

@lstein
Copy link
Collaborator

lstein commented Dec 3, 2022

I would be more comfortable with this PR if it weren't also changing a bunch of .py and .js files. We'll have to do some extended testing on this to make sure that there wasn't a whitespace change that inadvertently broke something.

@thinkyhead
Copy link
Contributor Author

Trailing whitespace removal is 100% safe and will have zero effect on the code. You'll notice that the regex ends in a $ which guarantees that it only replaces trailing whitespace. I do this regex replace all the time in the project I maintain, in that case with a mix of Python, C++, Javascript, shell scripts, etc. all in the same project, and it's just fine. Scan through the changes in the files tab above and you will see that all the changes are perfectly safe.

@thinkyhead thinkyhead force-pushed the whitespace_cleanup branch 3 times, most recently from 486ed64 to 1873ba7 Compare December 4, 2022 00:59
@thinkyhead
Copy link
Contributor Author

thinkyhead commented Dec 4, 2022

Did a quick rebase from main and rechecked to make sure no non-text files were affected, and no "patch" files were affected.

Copy link
Contributor

@mauwii mauwii left a comment

Choose a reason for hiding this comment

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

Generated some images via web and looked over the docs, seems to be fine.

Copy link
Collaborator

@lstein lstein left a comment

Choose a reason for hiding this comment

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

Thank you!

@lstein lstein enabled auto-merge (squash) December 19, 2022 05:05
@lstein lstein merged commit 7d8d4bc into invoke-ai:main Dec 19, 2022
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.

None yet

3 participants