Postponed on #1809930: [META] Many core class names violate naming standards

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because most fixed issues tagged coding standards are marked as tasks
Issue priority Not critical
Unfrozen changes Not Unfrozen
Disruption Changing classnames during beta phase would be very disruptive for core/contributed and custom modules/themes.

Problem/Motivation

As per discussion in #1809930: [META] Many core class names violate naming standards, many class names in the UUID namespace violate Drupal's naming standards (see http://drupal.org/node/608152). Fixing these class names to be more self-explanitory will help code be self-documenting and ease the introduction of new contributors.

Proposed resolution

Below is a list of classes Drupal/Component/Uuid namespace, along with the proposed changes. The attached .txt file is CSV, and contains the same information.

  • Com -> UUIDBackendCOM
  • Pecl -> UUIDBackendPECL
  • Php -> UUIDBackendPHP
  • Uuid
  • UuidInterface

User interface changes

None

API changes

(API changes/additions that would affect module, install profile, and theme developers, including examples of before/after code if appropriate)
Renaming classes in Core is likely to require large updates to existing patches and contributed modules. If we intend to enforce the naming standard, it should be done as soon as possible.

CommentFileSizeAuthor
#5 uuid_clasnames-1815988-5.patch35.45 KBthreewestwinds
#4 uuid_clasnames-1815988-4.patch35.4 KBthreewestwinds
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mitchell’s picture

mitchell CreditAttribution: mitchell commented
Issue tags: +UUID

(tagging)

skwashd’s picture

skwashd CreditAttribution: skwashd commented

I'd disagree with this. The whole idea of namespacing is that you don't need to have massive class names. I'm worried we are going to end up with something like this in Drupal http://javadoc.bugaco.com/com/sun/java/swing/plaf/nimbus/InternalFrameIn... (link credit: @msonnabaum)

My comment probably belongs on the naming convention issue, but I found while searching for UUID issues, so I've left it here.

threewestwinds’s picture

threewestwinds CreditAttribution: threewestwinds commented

To quote jhodgon from the Meta issue linked up top, "That standard has already been adopted, sorry." At this point, it's decided on, and this issue to to enforce the decision, not make it.

That's not to say the linked you attached isn't made of pure horror, and I'd hate to see that sort of thing appear in Drupal. :O

If you wanted to address it, then I'd suggest either opening a new issue to change the naming conventions or reopen #1507828: [policy, no patch] Revised standards for class naming within namespaces, where the current convention was discussed and decided upon.

threewestwinds’s picture

threewestwinds CreditAttribution: threewestwinds commented
Status: Active » Needs review
FileSize
uuid_clasnames-1815988-4.patch35.4 KB

Here is a first stab at a patch. Let's see how badly I broke everything.

threewestwinds’s picture

threewestwinds CreditAttribution: threewestwinds commented
FileSize
uuid_clasnames-1815988-5.patch35.45 KB

Oops, that patch file didn't include quite all of the changes it should have (missed a few Pecl -> UUIDBackendPECL changes). Let's hope this one is a bit better.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

We need to resolve #1627350: Patch for: Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName) soon! (That has to do with case of acronyms in class names).

But that aside... When I look at the UUID classes, I see this.

The main class is currently called "Uuid", and it's documented as "Factory class for UUIDs". So, it might make more sense to call it UuidFactory? Just calling it "Uuid" doesn't really tell what it does.... Hmm. Well, as I read the documentation further, it's not actually a just a factory class in the normal sense of the term Factory
http://en.wikipedia.org/wiki/Factory_(software_concept)
So really... scratch that. I think the class name Uuid is good (plus or minus a capitalization change), and the documentation should be changed so it doesn't say it's a factory class. It isn't.

OK, moving on... The member variable $plugin on that class (which is of type UuidInterface), says "Holds the UUID implementation", and the documentation on the Uuid class also mentions "the UUID implementation" in other places. So I think maybe the interface class should be called UuidImplementationInterface, and the 3 specific implementations should have "Implementation" rather than "Backend" in their names? Otherwise, the documentation on the Uuid factory class should be changed to refer to "backends" instead of "implementations", but I think probably "implementation" is more accurate.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

See also #1924818: Things that are not really factories should not be documented as factories

threewestwinds’s picture

threewestwinds CreditAttribution: threewestwinds commented

I was not aware of that issue on changing the naming standard - I'll hold off on having another go at this patch until we have our standard settled.

I disagree with Backend -> Implementation though - I don't think that implementation describes what's happening any better, and I'm against adding syllables unless there's a descriptive gain. Our classes are not Uuid implementations - they're pointers that know how to talk to the actual implementations. In this case, I'd say the documentation is what should be changed.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

I'm OK with changing the documentation in that case. What I'd like to avoid is having the docs say one thing and the class names (which are really kind of documentation too) say something else. And I still think that the interface should have the word "backend" or "implementation" in it (same as the backend/implementation classes) -- otherwise it looks like it's an interface for the overall Uuid class, which is not correct.

manningpete’s picture

manningpete CreditAttribution: manningpete commented
Version: 8.0.x-dev » 8.1.x-dev
Category: Bug report » Task
Issue summary: View changes
Status: Needs work » Postponed

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Component: base system » other
Issue summary: View changes
Parent issue: » #1809930: [META] Many core class names violate naming standards

Postponed on parent.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Issue tags: +Coding standards

Add tag

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Postponed » Closed (works as designed)

The naming convention change in March 2013.

If an acronym is used in a class or method name, make it CamelCase too (SampleXmlClass, not SampleXMLClass). [Note: this standard was adopted in March 2013, reversing the previous standard.]