Closed (outdated)
Project:
Drupal core
Version:
9.5.x-dev
Component:
documentation
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
1 Aug 2021 at 06:06 UTC
Updated:
2 Oct 2022 at 11:20 UTC
Jump to comment: Most recent
Show commands
changes
Comments
Comment #2
spokjeComment #3
spokjeComment #5
spokjeLet's see what TestBot thinks so far.
Comment #6
quietone commentedShouldn't this be done in a sniff?
Adding related issue that has a patch. Maybe a duplicate - I haven't checked.
Comment #7
spokjeThanks @quietone for finding the related issue, it's more a subset and also _very_ old. I think we can close that one? Not too sure if that would be due to being a duplicate or outdated?
> Shouldn't this be done in a sniff?
Good point, would however realistically mean this won't be done within the next 6 months looking at the lack of traction in the Coder issue queue and the fact that it would require a new release of Coder and getting that release into Cores dependencies.
Since I and reviewers have been burned before by putting (a lot of) time in bringing a non-sniff issue to RTBC only to be put back to postponed in the end for the lack of a sniff, let's postpone this one on approval of some Big Brains.
Comment #8
quietone commentedI wasn't sure if the changes here are for PhpStorm or for api.drupal.org so I picked a change to a migrate file to look at. The change here adds a leading slash to the @covers in core/modules/migrate/tests/src/Unit/process/FormatDateTest.php. On api.drupal.org that is correctly converted to a link to the source file. Maybe there are other changes that will improve api.drupal.org?
Possible coding standards issue #2341405: Decide on standard for referencing namespaced classes.
@Spokje, I ran the script in #2030977-5: Namespaces in documentation need leading \ and it created a 373K patch and on looking at the changes it it over zealous, which is pointed out later in #9. Since that is the older issue, this one should be made the duplicate and work/credit moved there. But before any of that let's get agreement on whether this should be done by a sniff or not.
Comment #9
quietone commentedOooh, another related.
Comment #10
spokjeComment #13
spokjeOk, took me a while to circle back to this one.
So, the way I see it (which can be totally wrong) #2030977: Namespaces in documentation need leading \ is a kind of "Fix-All-The-Things!" related to namespaces not having a leading backslash.
This issue is "only" about incorrect namespaces in
@tags(like@var,@see,@return).Within this issue there are basically two categories:
I agree with being burned before on non-sniff changes, so I created a sniff for 2. here: #3281300: @param tags must use fully-qualified class names, even when the full namespace is given without the leading backslash..
That sniff (if it is approved) will only fix a few of our issues (currently 22 to be exact), but the added value is that the changes are validated against the namespaces in the
use-block of the Class.That gives us the certainty that, at the moment of a sniff error and fix, the changed namespace actually exists.
I don't think any sniff/script can do that for all the missing backslash namespaces that float around, so IMHO although this is just a small improvement, the changes that are made with the sniff are, at the moment of changing, checked, which is a (small) win.
Let's await the approval of the sniff first, and landing of a version of
drupal/coderthat contains that sniff in Drupal Core first, before continuing here.If it lands, there will be plenty un-sniffed changes left, but less is more!
Comment #14
spokjePostponing on at least landing of one sniff: #3281300: @param tags must use fully-qualified class names, even when the full namespace is given without the leading backslash.
Comment #15
spokje