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

Namespace change from Dealerdirect to PHPCSStandards Was NOT Clearly Signalled as a Breaking Change #205

Closed
1 task done
timnolte opened this issue Feb 23, 2023 · 3 comments

Comments

@timnolte
Copy link

timnolte commented Feb 23, 2023

Problem/Motivation

Nothing in the README or the Release notes clearly indicated a BREAKING CHANGE to the namespace rename. Upgrading the package and trying to look through documentation to find what might have change proved almost impossible. The README needs to clearly indicate with it's own major heading and highlighting that this change was made. To be fair, I recognize that this release by nature of it being a major SemVer release could be taken that way though in this day and age it's not always the case. ;-)

Expected behaviour

Reading through the README or the Release notes should have very clear BREAKING CHANGE notices to help people fix the issue of failing runs due this significant change.

Actual behaviour

Class Dealerdirect\Composer\Plugin\Installers\PHPCodeSniffer\Plugin is not autoloadable, can not call install-codestandards script

And since the package name didn't change it is not obvious right away without a lot digging what happened.

Steps to reproduce

  1. Upgrade to v1 of the package.
  2. A composer.json script that runs Dealerdirect\\Composer\\Plugin\\Installers\\PHPCodeSniffer\\Plugin::run after package install/update.
  3. The installer isn't ran and returns an error.

Proposed changes

(If you have a proposed change, workaround or fix, describe the rationale behind it)

In the README section for "Calling the plugin directly" this namespace change should be clearly called out.

Environment

Question Answer
OS Linux
PHP version 8.0.27
Composer version 2.2.9
PHP_CodeSniffer version 3.7.1
Composer PHPCS plugin version 1.0.0
Install type Composer project local

Output of vendor/bin/phpcs --config-show:

> public/wp-content/vendor/bin/phpcs '--config-show'
Using config file: /var/www/html/public/wp-content/vendor/squizlabs/php_codesniffer/CodeSniffer.conf

installed_paths: ../../phpcsstandards/phpcsextra,../../phpcsstandards/phpcsutils,../../wp-coding-standards/wpcs

Tested against main branch?

  • I have verified the issue still exists in the main branch.
@timnolte timnolte changed the title Namesspace change from Dealerdirect to PHPCSStandards Was NOT Clearly Signalled as a Breaking Change Namespace change from Dealerdirect to PHPCSStandards Was NOT Clearly Signalled as a Breaking Change Feb 23, 2023
@Potherca
Copy link
Member

Potherca commented Feb 23, 2023

Thank you for taking the time to report this.

Looks like we dropped the ball on this one 😞

I've updated the release notes for the v1 release to clearly state the backward-breaking change.

I'm not 100% convinced change needs to be made to the README... @jrfnl do you have opinions on the matter?

@timnolte
Copy link
Author

eh, you obviously had a pretty big release on your hands so these things can happen. Thanks for updating the release notes! 👍🏼

@jrfnl
Copy link
Member

jrfnl commented Feb 23, 2023

The README was updated in the same PR as the namespace rename.

I personally don't think that the README needs to document outdated usage, so IMO, the README as-is, is fine. Especially considering every previous release was a pre-1.0 release and 1.0 should be the "truth" going forward.

I do very much agree that this change should have been highlighted more clearly in the changelog/release notes. Thank you @timnolte for calling that out and @Potherca for making the update!

@Potherca Should the branch rename from master to main maybe also be listed with the breaking changes ?

@jrfnl jrfnl closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants