Skip to content

[5.4] Adds code style fixes and phpstan update#47936

Open
laoneo wants to merge 2 commits into
joomla:5.4-devfrom
Digital-Peak:ci/portback
Open

[5.4] Adds code style fixes and phpstan update#47936
laoneo wants to merge 2 commits into
joomla:5.4-devfrom
Digital-Peak:ci/portback

Conversation

@laoneo

@laoneo laoneo commented Jun 11, 2026

Copy link
Copy Markdown
Member
  • I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.

Summary of Changes

Porting back some changes from the 6.2 branch and additionally #47914. Related pr's are:

Testing Instructions

Code review.

Link to documentations

Please select:

  • Documentation link for guide.joomla.org:

  • No documentation changes for guide.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@Hackwar

Hackwar commented Jun 11, 2026

Copy link
Copy Markdown
Member

Can we please add a message in the respective PR that we updated the codestyle and/or baseline in each case? Users should be informed that their code has been changed.

@Hackwar

Hackwar commented Jun 11, 2026

Copy link
Copy Markdown
Member

And since we are dropping phpcs with this PR from the CI, can we remove it entirely? See the changes from #45932

@laoneo

laoneo commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

Can we please add a message in the respective PR that we updated the codestyle and/or baseline in each case? Users should be informed that their code has been changed.

I think this is not needed as there is a new commit from the bot where they get notified.

And since we are dropping phpcs with this PR from the CI, can we remove it entirely? See the changes from #45932

This should then be done in 7.0 or 6.2 if really needed. But definitely not in 5.4.

@Hackwar

Hackwar commented Jun 11, 2026

Copy link
Copy Markdown
Member

I've gotten close to a hundred mails from Github in the 11 days of this month so far and I generally skip over the commit messages in a PR. I would not notice the commit from a bot between all those other messages and notices. I strongly suggest adding a comment that we did this over the head of the developer.

@laoneo

laoneo commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

And what would you do differently when you get a mail with the extra comment beside the commit mail?

@brianteeman

Copy link
Copy Markdown
Contributor

I'm with @Hackwar on this. At least on my own PR I would want to know. How else will I learn for next time

@laoneo

laoneo commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

I guess you missed the intention for this. This is not something the pr developer has actually to care about and learn. It is something we as a project require and enforce with this pr. As a pr author you should not care about code style and just write your improvement. This is not something you have to learn. This pr is made that you can forget about code style and focus on your work.

@brianteeman

Copy link
Copy Markdown
Contributor

I would rather learn to do it right then have it fixed silently. You don't learn to spell by having your work in one place corrected silently. Not everywhere I write will have the spell checker enabled.

@laoneo

laoneo commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

I leave it like this, if the project wants it, then take it as is, otherwise close the pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants