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

Preserve empty lines when using spoom bump #124

Merged

Conversation

bravehager
Copy link
Contributor

@bravehager bravehager commented Mar 31, 2022

Fix for #118

Right now, Spoom will remove empty lines after bumping files. My guess is most codebases wouldn't be affected because of enforced magic comments such as # frozen_string_literal: true.

Before:

# typed: false

class Foo; end

After:

# typed: true
class Foo; end

Adjusting the type sigil regex to only match on horizontal whitespace (tabs and spaces) seems to fix the issue, although there could be additional edge cases to consider.

Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Looks good to me, only one small suggestion about documentation and we can get this merged 👍

lib/spoom/test_helpers/project.rb Show resolved Hide resolved
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
@bravehager bravehager requested a review from Morriar March 31, 2022 19:38
Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Amazing, thanks!

@Morriar Morriar self-assigned this Apr 1, 2022
@Morriar Morriar merged commit 4ea0a21 into Shopify:main Apr 4, 2022
@shopify-shipit shopify-shipit bot temporarily deployed to production April 4, 2022 19:32 Inactive
@Morriar
Copy link
Collaborator

Morriar commented Apr 4, 2022

@bravehager I just released spoom 1.1.10 with your fix 👍

Thanks you again 🙏

@Morriar Morriar added the bugfix Fix a bug label Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants