Skip to content

[Config] Handle $rectorConfig->import() with wildcards * config#4832

Merged
TomasVotruba merged 4 commits intomainfrom
wilcards-import
Aug 23, 2023
Merged

[Config] Handle $rectorConfig->import() with wildcards * config#4832
TomasVotruba merged 4 commits intomainfrom
wilcards-import

Conversation

@samsonasik
Copy link
Copy Markdown
Member

Fixes rectorphp/rector#8151

Hi @rmobis, this should handle $rectorConfig->import() with wildcards * config. Could you verify this? Thank you.

@samsonasik
Copy link
Copy Markdown
Member Author

@TomasVotruba I added e2e test for wilcards path import config to prove it :) 43d4bbf

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready.

@rmobis
Copy link
Copy Markdown

rmobis commented Aug 23, 2023

@samsonasik I'm afraid the current change doesn't seem to fix the issue completely, only the somewhat incomplete MRE I had presented. I've updated the repository and the issue with appropriate description and instructions to reproduce, but in short the proposed code only imports one file matching the wildcard and not all of them, which is the expected behavior.

PS: I could not find a good way to directly import the branch as a composer dependency (I'm guessing because there's a build step?), so I applied the very small changes manually.

@samsonasik
Copy link
Copy Markdown
Member Author

@rmobis @staabm I updated to allow imports multiple configs resolved by glob() eae8bf6

@samsonasik
Copy link
Copy Markdown
Member Author

@TomasVotruba @rmobis @staabm I updated e2e test for multi configs included test for the prove :) 29cae3d

@rmobis
Copy link
Copy Markdown

rmobis commented Aug 23, 2023

Seems to be working now. Thanks for the quick fix.

@TomasVotruba
Copy link
Copy Markdown
Member

You change my mind with easyfix :) ... Let's ship it then 👍

@TomasVotruba
Copy link
Copy Markdown
Member

In hindsight, we're moving away from magic * paths and here is not much gain in making API so open.

Let's keep Rector simple and explicit, like other PHP CLI tools are 👍

Reverting here: #5010

@rmobis
Copy link
Copy Markdown

rmobis commented Sep 13, 2023

@TomasVotruba Fair enough. However, may I suggest we add a mention to this breaking change in the release notes? Either 0.18 or 0.18.3. Just so that people can easily find out why a previously working behavior now breaks.

@TomasVotruba
Copy link
Copy Markdown
Member

I'll update the 0.18 notes, thanks for feedback

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.

RectorConfig::import no longer supports globs after 0.18

4 participants