Skip to content

Conversation

nicodemuz
Copy link

Description

Ported this PR from squizlabs/PHP_CodeSniffer#2918

Suggested changelog entry

Add a new report for GitHub Actions annotations

Related issues/external references

Fixes #

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added XML documentation for the sniff.
  • I have opened a sister-PR in the documentation repository to update the Wiki.

@jrfnl
Copy link
Member

jrfnl commented Aug 12, 2025

Closing as violating another person's copyright and clearly not having read (or understood) the contributing guide which explicitly forbids this.

@nicodemuz
Copy link
Author

@jrfnl Apologies for this. My intention was not to violate anyone's copyrights. I even mention in my PR that the code was ported from another PR (created by @BrianHenryIE) towards the old repo. The licensing does mention that:

By contributing code to this repository, you agree to license your code for use under the BSD-3-Clause license.

As such, I was assuming that @BrianHenryIE was already granting and agreeing that his code could be used for PHP_CodeSniffer. If @BrianHenryIE provides his blessing for this PR to be merged, could we reopen this PR?

If not, would you be open for me to create a new PR from scratch that implements the same feature?

@jrfnl
Copy link
Member

jrfnl commented Aug 16, 2025

@jrfnl Apologies for this. My intention was not to violate anyone's copyrights. I even mention in my PR that the code was ported from another PR (created by @BrianHenryIE) towards the old repo. The licensing does mention that:

By contributing code to this repository, you agree to license your code for use under the BSD-3-Clause license.

As such, I was assuming that @BrianHenryIE was already granting and agreeing that his code could be used for PHP_CodeSniffer. If @BrianHenryIE provides his blessing for this PR to be merged, could we reopen this PR?

Sorry, but no. Copying the code and presenting it as your own without the correct authorship is not acceptable.
You could have asked @BrianHenry to port over the PR, you could have cherrypicked the commit(s) from the original PR, you could even have used the co-author tag in the commit message. This PR does none of those things and on top of that you check the license checkbox, which even explicitly asks whether you have the right to do so, which you clearly don't as it is not your code.

If not, would you be open for me to create a new PR from scratch that implements the same feature?

By now, GH annotations is a solved problem with the cs2pr tooling (see the example workflow code in a comment on the previous PR), so I don't see what value adding such a report would add.

@nicodemuz
Copy link
Author

Sorry, but no. Copying the code and presenting it as your own without the correct authorship is not acceptable.

You could have asked @BrianHenry to port over the PR, you could have cherrypicked the commit(s) from the original PR, you could even have used the co-author tag in the commit message. This PR does none of those things and on top of that you check the license checkbox, which even explicitly asks whether you have the right to do so, which you clearly don't as it is not your code.

My apologies, again. I did leave his name as an author in the source code which I (falsely) assumed would have been sufficient. Again, my bad. If you'd like, I can issue another PR that issues the commits in a fashion that you mentioned. 🙂

By now, GH annotations is a solved problem with the cs2pr tooling (see the example workflow code in a comment on the previous PR), so I don't see what value adding such a report would add.

I'm aware of this, as was the original author. Unfortunately, our organization doesn't like to add any unnecessary dependencies.

Also, other tools such as Psalm, PhpStan and Phan support this output format natively, so I was assuming it would be a welcome addition to PHP CodeSniffer.

@jrfnl
Copy link
Member

jrfnl commented Aug 16, 2025

Unfortunately, our organization doesn't like to add any unnecessary dependencies.

I'd say that if you want annotations in GitHub, it's not an unnecessary dependency.

Also, other tools such as Psalm, PhpStan and Phan support this output format natively, so I was assuming it would be a welcome addition to PHP CodeSniffer.

What other tools do is not an argument.

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.

2 participants