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

don't associate a file with a different package that matches a prefix #398

Merged

Conversation

exterm
Copy link
Contributor

@exterm exterm commented Mar 19, 2024

What are you trying to accomplish?

Fix #297

What approach did you choose and why?

I think technically we shouldn't do string comparison for this, but the fix was so easy, and swapping in Pathname everywhere quite a bit of work, so I opted for this variant

What should reviewers focus on?

Type of Change

  • Bugfix
  • New feature
  • Non-breaking change (a change that doesn't alter functionality - i.e., code refactor, configs, etc.)

Additional Release Notes

  • Breaking change (fix or feature that would cause existing functionality to change)
    This may reattribute files to other packages (the correct packages) and thus add or remove violations.

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • It is safe to rollback this change.

Additional Notes

This work is sponsored by https://www.onemedical.com/

@exterm exterm requested a review from a team as a code owner March 19, 2024 16:34
@exterm exterm force-pushed the fix-package-mismatch-on-overlapping-prefix branch from f4d50ca to 133619a Compare March 19, 2024 16:38
@exterm exterm force-pushed the fix-package-mismatch-on-overlapping-prefix branch from 133619a to ca7ae0d Compare March 19, 2024 18:39
@exterm
Copy link
Contributor Author

exterm commented Apr 16, 2024

@gmcgibbon @shioyama can we get this straightforward bugfix merged?

@exterm
Copy link
Contributor Author

exterm commented May 4, 2024

@rafaelfranca I see you've been active on this repo recently - can you give this small bugfix a review?

@gmcgibbon gmcgibbon merged commit d3584ce into Shopify:main May 7, 2024
20 checks passed
@gmcgibbon
Copy link
Member

Thanks, sorry for the delay!

@seanarnold
Copy link

@gmcgibbon do you mind cutting a release with this included?

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.

Packwerk thinks that app/helpers/foobar_fizz.rb is in the package app/helpers/foobar and not app/helpers
3 participants