Problem/Motivation

We can observe in drupal-composer/drupal-project that the files .gitattributes and .editorconfig are relocated to the project root:

        "drupal-scaffold": {
            "initial": {
                ".editorconfig": "../.editorconfig",
                ".gitattributes": "../.gitattributes"
            }
        }

This relocation makes sense, as these files work best when placed at the project root.

Proposed resolution

Define a new location in drupal/core-composer-scaffold, [project-root]. Update the scaffold assets to place these two files at the project root rather than the web root.

Remaining tasks

Write up a change record.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

When building a Composer-managed Drupal site with drupal/recommended-project, the files .gitattributes and .editorconfig are now placed at the project root instead of the Drupal root.

CommentFileSizeAuthor
#13 interdiff-10-13.txt653 byteshussainweb
#13 3084326-13.patch3.92 KBhussainweb
#11 interdiff-5-10.txt653 byteshussainweb
#11 3084326-10.patch6.77 KBhussainweb
#5 3084326-5.patch6.53 KBgreg.1.anderson
#5 3084326-2-to-5-interdiff.txt5.03 KBgreg.1.anderson
#2 3084326.patch1.5 KBgreg.1.anderson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greg.1.anderson created an issue. See original summary.

greg.1.anderson’s picture

greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commented
Status: Active » Needs review
FileSize
3084326.patch1.5 KB

Here's the patch.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Needs review » Reviewed & tested by the community

Also the right thing to do - drupal/drupal ships with these in the project root as well, so makes sense to put them there.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 3084326.patch, failed testing. View results

greg.1.anderson’s picture

greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commented
Status: Needs work » Needs review
FileSize
3084326-2-to-5-interdiff.txt5.03 KB
3084326-5.patch6.53 KB

Fix test failure.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Needs review » Reviewed & tested by the community

ah. I guess I forgot we had test coverage for those. Sorry for the hasty RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

Under remaining tasks it says

Write up a change record.

Does this need a new CR or can an existing one be extended?

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs change record

Since we've had templates in core for less than a week, I don't believe this is a significant enough change to warrant a new CR, I went ahead and added

Both template variants will place a .editorconfig file and a .gitattributes file in the root of your project, adjusted for Drupal coding standards.

to the existing CR: https://www.drupal.org/node/3082474

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/tests/Drupal/Tests/ComposerIntegrationTest.php
@@ -177,29 +177,29 @@ public function testAllModulesReplaced() {
+      ['.csslintrc', 'assets/scaffold/files/csslintrc', '[web-root]'],
+      ['.eslintignore', 'assets/scaffold/files/eslintignore', '[web-root]'],

@@ -224,14 +224,14 @@ public function providerTestExpectedScaffoldFiles() {
+  public function testExpectedScaffoldFiles($destRelPath, $sourceRelPath, $expectedDestination) {

is it worth defaulting $expectedDestination to [web-root] as this is where the bulk of the items live and are likely to in the future - might save us a few cycles down the track

greg.1.anderson’s picture

greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commented
FileSize
3084326-10.patch3.68 KB
3084326-5-to-10-interdiff.txt4.34 KB

Good idea. I almost did that the first time around.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
FileSize
3084326-10.patch6.77 KB
interdiff-5-10.txt653 bytes

> is it worth defaulting $expectedDestination to [web-root] as this is where the bulk of the items live and are likely to in the future

I am not very sure about this. Since this is a test method and not an API, I think it's worth being explicit rather than making it the default. I am not very strongly opinionated about this though.

Meanwhile, I am adding the param docblock for the new parameter in that method.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

Oops... didn't mean to save again

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
FileSize
3084326-13.patch3.92 KB
interdiff-10-13.txt653 bytes

Adding the docblock on top of patch in #10 again.

greg.1.anderson’s picture

greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commented
Status: Needs review » Reviewed & tested by the community

Review comments addressed, and tests back to green. Thanks, @hussainweb

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Committed c7a1b02 and pushed to 8.8.x. Thanks!

Thanks folks

  • larowlan committed c7a1b02 on 8.8.x 
    Issue #3084326 by hussainweb, greg.1.anderson: Install .editorconfig and...

Status: Fixed » Closed (fixed)

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