Comments

timmillwood’s picture

Status: Needs work » Needs review
StatusFileSize
new372 bytes

Status: Needs review » Needs work

The last submitted patch, 1: add_composer_json-2514620-1.patch, failed testing.

mile23’s picture

I say yay Composer! But I also wonder why we need this.

What specifically does it support?

timmillwood’s picture

The example module doesn't need it, but all modules should have it, so it's setting an example.

The reasoning for all modules having a composer.json is mainly to set a "type" and therefore get the module to go into the modules directory when installed via composer.

mile23’s picture

Cool. :-)

Aside from the broken HEAD problem making this fail.... :-)

The patch adds a whitespace line to the end of composer.json.

Also it doesn't pass when you say composer validate, needing a license section. Everything on D.O is GPL 2.0+

I'd also suggest we add a "_readme" section explaining why the file is there, saying essentially what you said above: Drupal 8 doesn't quite support Composer but it's possible to install modules where they belong with composer-installer and other techniques, and a link to documentation about how to do that.

timmillwood’s picture

Status: Needs work » Needs review
StatusFileSize
new1.04 KB

Added license to composer.json and a basic paragraph to README.

pp’s picture

StatusFileSize
new1.04 KB

I checked the file with composer validate and I got the following error message:

./composer.json is valid, but with a few warnings
See https://getcomposer.org/doc/04-schema.md for details on the schema
License "GPL 2.0+" is not a valid SPDX license identifier, see http://www.spdx.org/licenses/ if you use an open license.
If the software is closed-source, you may use "proprietary" as license.

The right licence id is "GPL-2.0+". I corrected it only, and rerolled the patch.

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!

mile23’s picture

Status: Reviewed & tested by the community » Needs work

So we have a root-level examples module, but we also have all the other modules. Do they need composer.json as well? This is one of the questions that hasn't been answered in the many Composer-related core issues.

Also, let's not dictate that contrib *should* have a composer.json file. Let's say that you *can* have one, and link to a documentation page about it, why it's a good idea, etc.

sethfischer’s picture

We should retain the wording "... all contrib modules *should* ship with one."

Using the word "must" would dictate that a composer.json is a mandatory requirement. The word "should" indicates that it is a good practice to include a composer.json - but it is not mandatory.

"Can" is the wrong word to use in this context. A module author can include any file they like in their module. Hence we don't need to give the authors permission to include a composer.json.

Developers are already using a composer based workflow for Drupal 8. We want modules to start shipping with a composer.json now to accommodate this.

timmillwood’s picture

Status: Needs work » Reviewed & tested by the community

"should" is the exact wording needed. Eventually we want to automate the listing of all D.o modules on packagist, without a composer.json then will not get listed.

mile23’s picture

Status: Reviewed & tested by the community » Fixed

OK, committed. Thanks, folks. :-)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.