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

feat: increase ordered_interfaces fixer priority #7761

Conversation

erickskrauch
Copy link
Contributor

Fixes #7760.

@coveralls
Copy link

coveralls commented Jan 18, 2024

Coverage Status

coverage: 94.755%. remained the same
when pulling 55f6c3f on erickskrauch:increase_ordered_interfaces_priority
into cff91c0 on PHP-CS-Fixer:master.

@erickskrauch
Copy link
Contributor Author

In case this PR will be accepted, I feel like there should be a comment which will describe the reason, why this priority is set.

@keradus
Copy link
Member

keradus commented Jan 19, 2024

@erickskrauch , can you fix the failing CI jobs and also sync master branch into your PR?

@erickskrauch
Copy link
Contributor Author

@keradus, I can't anymore. Since the changes introduced in the 9167592 commit, the ordered_interfaces must have priority <7, but for my case I need it to be at least 67, which is invariant.

If you don't see any possible solution for this situation, I think I will need to close this PR as not implementable at the moment until some graph-based priorities are introduced.

@keradus
Copy link
Member

keradus commented Jan 20, 2024

The mentioned commit proved priority dependency between 2 fixers that was not explicitly declared before. That not-yet-discovered priority dependencies are the risk I was mentioning for changing any priority.

I see the value in switching to graph solution, but if you would construct the graph with circuit (A->B->C->A), and to what I understand is your challenge, it would not allow us to solve and run fixers in linear order. I'm happy to have a PR one day that is not only moving us to graph, but also solving the circuits.

I'm closing the PR due to discussion.

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.

Increase priority of the ordered_interfaces fixer
3 participants