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

allow setting patches file location through --patches-file option (fixes #25) #26

Merged
merged 6 commits into from
Nov 17, 2024

Conversation

rmobis
Copy link
Contributor

@rmobis rmobis commented Sep 25, 2024

Adds --patches-file option which allows using a patches file instead of relying on composer.json.

Since the structure for the patches.json is "shallower" (no extra at the top) the code path is a bit different. Could have done a slightly bigger refactor, but I like to try and keep things simple when contributing to someone else's code.

When --patches-file is set we assume this shallower format, meaning you can't call with --patches-file=composer.json. Could instead check the file name at runtime and decide what format to use, but this was simpler and I think it makes sense. Let me know what you think.

Also add a test for changes.

@rmobis
Copy link
Contributor Author

rmobis commented Sep 25, 2024

I'm not really sure what the issue with rector is, it wasn't working for me locally even before I made any changes either. If that's something I did wrong, please let me know.

@samsonasik
Copy link
Member

Rector in composer.json still on 0.18.3

"rector/rector": "^0.18.13",

Could you try upgrade to ^1.2.5 ? Thank you.

@rmobis rmobis force-pushed the feature/patches-file branch from 7c56344 to 2907192 Compare September 25, 2024 17:32
@rmobis
Copy link
Contributor Author

rmobis commented Sep 25, 2024

Please don't merge, I just noticed there's a bug. I'll fix it in a couple of hours when I back.

@rmobis
Copy link
Contributor Author

rmobis commented Sep 27, 2024

Fixes have been applied, sorry for the delay. Let me know if there's anything else missing.

@rmobis
Copy link
Contributor Author

rmobis commented Nov 17, 2024

@TomasVotruba is anything missing for this to be merged?

@TomasVotruba
Copy link
Member

All good. Could you update the readme as well?

@rmobis
Copy link
Contributor Author

rmobis commented Nov 17, 2024

All good. Could you update the readme as well?

Done

@TomasVotruba TomasVotruba enabled auto-merge (squash) November 17, 2024 11:54
@TomasVotruba
Copy link
Member

Thank you 😊

@TomasVotruba TomasVotruba merged commit 085f53b into symplify:main Nov 17, 2024
7 checks passed
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.

3 participants