Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Beta phase evaluation
Issue category | Task: Follow up to an earlier task |
---|---|
Issue priority | Normal |
Unfrozen changes | Unfrozen because it only changes documentation/example code. |
Disruption | None. |
#2326913: Add sites/*/*services*.yml to example.gitignore was supposed to have added sites/*/*services*.yml
to example.gitignore
but it only added sites/*/services*.yml
. Ditto settings.php
. Let's finish the job. :)
Comment | File | Size | Author |
---|---|---|---|
#54 | interdiff_37-53.txt | 827 bytes | ankit_rathore |
#54 | example_gitignore-2381091-53.patch | 1.73 KB | ankit_rathore |
| |||
#52 | 3203420-52.patch | 827 bytes | Rassoni |
#37 | example_gitignore-2381091-37.patch | 825 bytes | sidharthap |
#32 | example_gitignore-2381091-32.patch | 821 bytes | gnuget |
Comments
Comment #1
mikeker CreditAttribution: mikeker commentedComment #2
mikeker CreditAttribution: mikeker commentedDang it... I never remember to set the status!
Comment #3
mikeker CreditAttribution: mikeker commentedLooks like I, and those in #2326913: Add sites/*/*services*.yml to example.gitignore, missed that we need to make this change to the commented out section of the example gitignore file as well... Bumping to priority to normal since the current
example.gitignore
doesn't include anything aboutservices.yml
files if the.gitignore
is located in the sites directory.Comment #4
dawehnerWell, I would argue that you maybe actually want to have some of those in GIT, especially if you have production files, you want to track as well.
Excluding all of them by default is maybe a little bit too much, to be honest.
Comment #5
mikeker CreditAttribution: mikeker commented@dawehner: Agreed -- I usually checking settings.php into the repo. But this is just an example
.gitignore
file and meant to be modified. Since there is no longer a.gitignore
in Drupal core there is no worry about it being overwritten with each core update. The hope is that someone new to Drupal can just copy/paste the example file and not accidentally check-in potentially sensitive information. More experienced devs will modify it to suit their needs.This also treats
local.settings.php
the same assettings.local.php
. Previously onlysettings.local.php
was ignored.Comment #6
dawehnerMaybe we could explicit allow
example.*services*.yml
andexample.*settings*.yml
which itself is an example what you want to have in your repo?Comment #7
mikeker CreditAttribution: mikeker commentedReroll after #1975220: Allow a Composer user to manage Drupal, modules, and PHP dependencies with a custom root composer.json.
re: #6: we have a
default.settings.php
anddefault.services.php
along withexample.gitignore
. I would support renaming the first two to start with example, just for consistency and the fact that neither file is actually used as a default anything. But I think that's a debate that'll be postponed until D9 (at the earliest! :)Comment #8
mikeker CreditAttribution: mikeker commentedPatch still applies...
Comment #9
dawehnerLet's get it in.
Comment #10
Wim Leerss/php/yml/ … or am I mistaken?
Comment #11
mikeker CreditAttribution: mikeker commented@Wim Leers: Not mistaken, completely correct!
Fixed in the attached patch.
Comment #12
Wim LeersLovely :)
Comment #13
alexpottIsn't a problem with this that (for example) default.settings.php is in git?
Comment #14
mikeker CreditAttribution: mikeker commentedNot if those files are already in the repo. The .gitignore file only ignores new files -- from the Git manpages:
So a core contributor wanting to change
default.settings.php
would see their change when they rungit status
. If we wanted to add anexample.settings.php
file, it would require overriding our existing.gitignore
to do so, but core contributors should be comfortable doing so and, since.gitignore
is not a tracked file, it would not appear in the diff.Comment #15
jeanfei CreditAttribution: jeanfei commentedI think it should be better to specify that the default.settings.php is in git.
I've add new lines in example.gitignore for this.
Comment #16
mikeker CreditAttribution: mikeker as a volunteer commented@jeanfei, agreed, specifically adding default.settings.php makes it clearer what is and is not in the repo. Thanks for the addition.
Patch still applies cleanly to HEAD. Added beta eval to the issue summary and, since this is a followup to a task, I changed issue type.
Comment #17
mikeker CreditAttribution: mikeker as a volunteer commentedComment #18
joelpittetLooks good a bit confusing based on the title which the patch addresses but doesn't but does:)
Comment #19
alexpottLet's do the same for default.services.yml as we've done for default.settings.php
Comment #20
mikeker CreditAttribution: mikeker as a volunteer commentedDone.
Comment #21
Wim Leersdefault.services.yml, not default.services.php.
Comment #22
mikeker CreditAttribution: mikeker as a volunteer commentedYikes! Sorry about that -- pre-coffee copy/pasting is dangerous... :)
Fixed.
Comment #24
sasanikolic CreditAttribution: sasanikolic as a volunteer commentedDid we forget
development.services.yml
? I have it insites/development.services.yml
... or should I move it into sites/default?Comment #25
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commentedHi
I have kept development.services.yml in sites folder as it is in drupal 8.1.x.
Just added the path sites/development.services.yml in git ignore. I had just given a try.
Please review.
Thanks!!
Comment #27
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commentedComment #29
lluvigneRerolled patch on #22 (i think that patch on #25 is not ok, maybe with this you can make the change, i don't know).
Comment #31
gnugetThis still doesn't address #24.
whether we have or not to move the development.services.yml to site/default the default place of that file is /sites/ and IMO we should ignore it there by default.
Comment #32
gnugetLet's see if this can push this forward.
Comment #33
shadcn CreditAttribution: shadcn commentedWouldn't this line here ignore modules' services.yml files in a multisite configuration?
Comment #34
gnugetYup, you are right. that line is even going to ignore /sites/default/services.yml file :-|
We need add an extra line there with something like:
Thanks for your review, this definitely needs work.
Comment #35
gnugetComment #36
xjmComment #37
sidharthapUpdated patch.
Comment #38
gnugetHi sidharthap.
Thanks for your patch, when a new patch is uploaded is necessary provide the interdiff as well.
Can you please upload it?
Thanks!
Comment #39
gnugetI'm confuse.
Initially I thougth to we wanted to ignore all the files but settings.php and services.yml but it seems to I was wrong, we wanted to ignore all of them but example.settings.php and example.services.yml?
If that is the case then: 32 is correct.
Comment #41
joelpittetIt seems that this issue title and summary doesn't match. Please update the title and summary to explain what this issue has evolved into.
Comment #52
Rassoni#37 is not applied not 10.x. version. no changes in patch just re-roll in 10.x.
Comment #54
ankit_rathore CreditAttribution: ankit_rathore at OpenSense Labs for DrupalFit commented#52 failed because only changing
/example.gitignore
will not work, patch will work after adding the same lines in
/core/assets/scaffold/files/example.gitignore
I am uploading updated patch for 10.1.x-dev with interdiff
Comment #55
smustgrave CreditAttribution: smustgrave at Mobomo commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue under control, following Review a patch or merge request as a guide.
This appears to need an issue summary as there seem to be more changes then what's originally listed. Using the default template would be super helpful.
===========
Just FYI to help get the message out there.
Starting March 2023, simple rerolls, rebases, or merges will no longer receive issue credit. Only rerolls that address a merge conflict will be credited, and the merge conflict that was resolved must be documented in the text of an issue comment.
To receive credit for contributing to this issue, assist with other outstanding tasks or unaddressed feedback.
See the issue credit guidelines for more information.