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.
- Attached patch clarifies on the real DX problems that everyone happens to ignore.
- Comments are intentionally written emotional and borderline offending.
- ???????
- Profit.
Comment | File | Size | Author |
---|---|---|---|
dx1.0.patch | 24.15 KB | sun | |
Comments
Comment #1
RobLoachWould be great to get some individual issues open to discuss some of these. While some are truly valid, I'm sure there are historic reason for others that are easily fixed.
Glad we have some direction.
Comment #2
thedavidmeister CreditAttribution: thedavidmeister commented+ #
Trailing white space.
Comment #3
thedavidmeister CreditAttribution: thedavidmeister commented@sun, maybe join the discussion/efforts at #2093143: [meta] Remove calls to @deprecated and "backwards compatibility" procedural functions from core. It might help address some part of your DX concerns.
Comment #4
dawehnerRelated issue: #2100511: Make routing.yml files have a delightful DX
Comment #5
jibranOther then the in progress route conversions I think rest of the points are valid. IMHO we honestly need some discussion to figure out some middle way. Like
String::
point, it is like this because of unit tests but I am sure we can figure something out.Comment #6
RobLoachIf it's the above you're referring to, it's just simply a typo. Should be
\Drupal\Component\Utility\Xss::filterAdmin()
. If we open up an issue for it, we'll be able to patch them individually.Comment #7
EclipseGc CreditAttribution: EclipseGc commentedSo, I'll see if I can justify a bit of this. (not all by any stretch, but just a few things)
1.) Use statements don't import anything, they just make a class available to be instantiated. Since we type hint on the short names, full use statements are used. This makes method signatures not look crazy, but does mean in some instances you end up with quite a few use statements.
2.) {@inheritdoc} can, and usually DOES, imply that documentation exists in an interface that method came from, not that you're overriding an existing method, though both cases are valid (I think).
3.) We had a long drawn out debate on the name "controller" in relation to entity stuff. I don't recall the actual resolution to that, but suffice it to say, we're not blind to all of these consistency issues. People are discussing changes and trying to fix it.
4.) You seem really hung up on the use statements. I'm not sure why this is. Perhaps you could elaborate about that topic a bit more and why it specifically bothers you? I'm happy to be helpful if possible.
5.) Your criticisms of route definition are pretty fair imo. I think there's a lot of dissatisfaction there, which is unfortunate, but we discussed this at length in Prague on numerous occasions. You're not alone, people are discussing and experimenting with some potential solutions.
All in all, I don't think I find any of this really surprising. I wish we'd had your voice to add to the discussion in Prague. I too, hope we can solve many of the issues you've pointed out. People ARE working on a lot of this stuff though.
Eclipse
Comment #8
damiankloip CreditAttribution: damiankloip commentedNope, This is just a method to check if the access checker applies to the route; NOT to check access.
Comment #9
thedavidmeister CreditAttribution: thedavidmeister commentedIt's nearly impossible to do or say anything constructive here as-is, and it looks like @sun put little to no effort into presenting his complaints in a constructive format, which is a real shame (and more offensive than the comments themselves). The current issue summary and attached patch aren't funny, at all, and it's unclear what it aims to accomplish, beyond propagating a general sense of disappointment.
Essentially, this issue in its current format is just trolling the community and wasting people's valuable time.
My preference would be that, if we're to continue doing anything at all here, we first:
- halt any further discussion until each part of the patch attached to the issue has been converted into a reference to a new/existing issue or actionable task in the summary so that we're all at least talking about the same thing(s).
- Identify a common underlying problem with all the comments in the patch in the summary and rewrite the issue title and summary so it's clear how they're all related beyond "critical-ness", "Drupal", "Sun" and "DX", comment #7 demonstrates that there's at least 5 independent complaints being raised here that are each different enough that it has become hard to see how they could all be "in scope" for a single meta discussion
- Identify which complaints in the patch are truly "critical" and which parts are really just a little bit inconvenient and were "swept up" into the general rant
Comment #11
Crell CreditAttribution: Crell commentedThis issue is not useful or actionable.
Comment #12
larowlanI think we need to at least ensure that the issues identified are covered in new or existing issues and are listed on the DX intiative page (https://drupal.org/node/2081777)
There are some minor documentation bugs that I have filed.
There are also some valid DX concerns, although we all agree that the delivery method doesn't help.
However I can accept that frustration is often expressed as sarcasm when you genuinely care so let's just let this go through to the keeper.
Created so far:
Already existed:
I think that covers most of it so updating status.
Have added #2104347: Unify argument order between \Drupal\Core\Entity\EntityAccessControllerInterface access and createAccess methods to the DX initiative page
Comment #13
jibranThank you @larowlan for the great work and sane reply.
Comment #14
rafamd CreditAttribution: rafamd commented^^^ indeed