Closed (fixed)
Project:
Coder
Version:
8.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
22 Feb 2017 at 07:38 UTC
Updated:
22 Mar 2017 at 14:04 UTC
Jump to comment: Most recent
"ERROR | All dependencies must be prefixed with the project name, for example "drupal:"
This is not true. In Drupal, the prefix for dependencies is OPTIONAL, not REQUIRED. At most, this should be a NOTICE, not an ERROR. See https://www.drupal.org/node/2299747
Comments
Comment #2
klausiComment #3
tr commentedComment #4
klausiOne problem with this sniff is that custom modules that depend on other custom modules cannot specify a prefix. That will cause false positives for people.
Which means we should move this sniff out of the Drupal standard into DrupalPractice which can have false positives. I still think the sniff is very useful, because Drupal core modules and all contrib projects on drupal.org should use namespaced dependencies.
So the "should" is really a very strong "should" and all drupal.org projects should follow it, for example all modules submitted in project applications. By moving it to DrupalPractice we should also downgrade the severity to be a warning instead of an error.
We have not released this sniff in a stable Coder release, so we can just move it without preserving compatibility.
Comment #6
klausiPushed that. Let me know if this is good enough or you have other ideas for improvements.
Comment #7
klausiThis sniff will also be used for Drupal core in #2821499: Enable phpcs rule DrupalPractice.InfoFiles.NamespacedDependency once the next stable Coder release is out (planned for March).
Comment #8
andypostOne more issue
Comment #9
tr commentedThe error showed up in the automated pareview of a project application on drupal.org, so is that using the development release? I wonder why you stress that this is for custom modules which depend on other custom modules - that isn't the case in the situation where I encountered this.
I want to avoid projects being delayed or denied by false positives - inexperienced reviewers don't always recognize that these are false positives, and applicants who are writing acceptable code are then told to change it (needlessly) if they want their project approved.
I also want to avoid a dual standard, where we have 1) the official documented coding standards which may be changed via community discussion, and 2) unofficial but de-facto standards imposed by the tools we use to check our code.
Comment #10
klausiYes, https://pareview.sh/ uses a dev version of Coder.
It is very important that drupal contrib maintainers prefix their dependencies because otherwise developers will spent countless hours figuring out why composer does weird things. Example: #2795709: Update composer.json and namespace module dependencies.
So for project applications this is a very valid warning that should always be fixed.
Comment #11
andypostThat's why I filed follow-up to limit this sniffer to modules only - core implementation works for them only
- #474684: Allow themes to declare dependencies on modules
- #2855026: Installation profiles do not support project:module format for dependencies