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

Install rector, add rector config, add rector changes, add rector for CI #17900

Open
wants to merge 1 commit into
base: 5.next
Choose a base branch
from

Conversation

Harfusha
Copy link
Contributor

No description provided.

@Harfusha
Copy link
Contributor Author

#17865

@ADmad
Copy link
Member

ADmad commented Sep 13, 2024

I am not in favor of adding rector as a dev dependency, nor running rector as a part of CI, that's wasteful.

These pedantic opioniated changes don't fix any actual problem nor bring any significant performance gains.

We can instead have a separate workflow which runs on a weekly basis and reports the diff or auto create a PR which we can review.

@dereuromark
Copy link
Member

We can also just composer require it at runtime directly inside the job.
Thats probably best.

@ADmad ADmad added this to the 5.1.0 milestone Sep 13, 2024
@othercorey
Copy link
Member

I would agree that rector is not our coding standard. We don't need another large static analyzer running on each commit.

@markstory markstory modified the milestones: 5.1.0, 5.1.1 Sep 14, 2024
@markstory markstory modified the milestones: 5.1.1, 5.1.2 Oct 4, 2024
@markstory markstory modified the milestones: 5.1.2, 5.1.3 Nov 10, 2024
@dereuromark
Copy link
Member

Do you want to refactor similar to other analyzer tasks to run only in ci? Without modifications to Code base?

@markstory markstory modified the milestones: 5.1.3, 5.1.4 Dec 13, 2024
@dereuromark
Copy link
Member

ping @Harfusha

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.

6 participants