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 inject into file warning #709

Merged

Conversation

nicolas-brousse
Copy link
Contributor

Not sure if the fix implementation is good enough. Let me know if you want some changes.

Closes #706.

Copy link

@dorner dorner left a comment

Choose a reason for hiding this comment

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

This looks good - I'd like to see this merged in!

@dorner
Copy link

dorner commented Jun 2, 2021

@nicolas-brousse can you rebase off master please?

@nicolas-brousse nicolas-brousse force-pushed the fix-inject-into-file-warning branch from 4ae8925 to bf3afab Compare June 3, 2021 10:02
Comment on lines 115 to 116
before, after = content.split(regexp, 2)
snippet = (behavior == :after ? after : before).to_s
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 code has been added. I've to found a way to deal this regexp.

Copy link

@dorner dorner 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! @rafaelfranca

@nicolas-brousse
Copy link
Contributor Author

For now when I tried locally, the test I added fail. I've to inspect how snippet = (behavior == :after ? after : before).to_s works. When I initially worked on this PR this line wasn't implemented yet.

@rafaelfranca
Copy link
Member

Tests are broken

@rafaelfranca rafaelfranca force-pushed the fix-inject-into-file-warning branch from bf3afab to 6499e07 Compare May 12, 2023 20:07
@rafaelfranca rafaelfranca merged commit 2dde972 into rails:main May 12, 2023
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.

insert_into_file print warning when change is already done
3 participants