It is becoming common for reviewers to include the existence of a composer.json that contains a drupal/core requirement as part of the reason to kick back a project to Needs work.

Not having a well formed composer.json is a Drupalism.

As noted in https://www.drupal.org/docs/develop/using-composer/add-a-composerjson-fi... there are times that the Facade is not utilized.

A well formed composer.json would include all its requirements including drupal/core.

Having a well formed composer.json may add a slight maintenance burden on a maintainer, however it allows their project to become a fully compliant member of the Composer ecosystem, this is not a trait that should be penalized.

Samples:

https://www.drupal.org/project/projectapplications/issues/3583592#commen...
https://www.drupal.org/project/projectapplications/issues/3583592#commen...
https://www.drupal.org/project/projectapplications/issues/3584486#commen...
https://www.drupal.org/project/projectapplications/issues/3577238#commen...
https://www.drupal.org/project/projectapplications/issues/3583244#commen...
(many many more).

Comments

cmlara created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Remove composer.json as a reason to send a projectapplications review back to needs work » Remove composer.json as a reason to set a security coverage application back to needs work
Status: Active » Postponed (maintainer needs more info)

None of those reviews is suggesting that a project should not have a composer.json file. They are suggesting removing the requirement for Drupal core, since the Composer Façade used on drupal.org adds that value basing on the core_version_requirement value contained in the .info.yml file.

No documentation requires that a project does not have a composer.json file. To what would Remove composer.json as a reason to set a security coverage application back to needs work apply?

cmlara’s picture

cmlara
he/him
commented
Title: Remove composer.json as a reason to set a security coverage application back to needs work » Remove having drupal/core present in composer.json as a reason to set a security coverage application back to needs work
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Active

No documentation requires that a project does not have a composer.json file. To what would Remove composer.json as a reason to set a security coverage application back to needs work apply?

At a very minimal, training a queue admin would inform reviewers in the issue when they flag an item that is invalid so they do not flag the same issue again in the future. It could be added to https://www.drupal.org/node/894256 or one of the sub-pages (I deffer to the maintainers to choose where they would prefer to document this).

They are suggesting removing the requirement for Drupal core, since the Composer Façade used on drupal.org adds that value basing on the core_version_requirement value contained in the .info.yml file.

Valid point, I've cleaned up the Issue Summery and title to better focus on the drupal/core aspect should not be penalized.

I previously linked to this phrasing:

The composer facade can not generate this metadata for GIT based checkouts that do not use the facade. As a maintainer to ensure compatibility with composer an entry for drupal/core may be required in your composer.json file.

https://www.drupal.org/docs/develop/using-composer/add-a-composerjson-file

As noted in the above the facade is not always utilized and as such a module may want to (and really good practice in the composer world is should) have a drupal/core version present otherwise Composer will be unable to determine the compatibility.

Examples of where this may occur:

  • Mirrored Repositories (local backups in case D.O. is offline, or large hosting providers reducing the load on D.O.)
  • Including the project as a Git Repository in the local composer.json (used for developing modules or testing merge requests)
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Active » Closed (won't fix)

In none of the linked applications the status has been changed to Needs work merely because the core requirements were duplicated in the composer.json file and in the .info.yml file. The reviews for those applications informed the applicants about what the Drupal.org Composer façade does; they could have been clearer about that not being a mandatory change, though.

None of the documentation pages for those applications says that avoiding duplicating core requirements the composer.json file and in the .info.yml file is mandatory for the application to be marked Fixed; none of those pages mentions that as one of the most requested changes, nor do they list a canonical comment to leave in that case. There is no documentation page that needs to be corrected.

The documentation pages for those applications cannot give a detailed list of points that should not be reported in a review, not at the point to list a single line in a file as line a review should not ask to change. There is no documentation page that needs to be edited.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.