Problem/Motivation

As we start to actually add code to this project, I'd like us to have a clear, standardized and rigorous plan for what we name things around here. Calling this parent issue / plan "Major" since it blocks progress on other issues.

In particular:

  • Plugin IDs
  • Config IDs
  • Class names

Some places this is already a starting to be a problem:

#2973035: Configurable list widget has a typo in its id
#2834016: Add 'Compact' datetime range formatter

It's also possible that "we" don't like the way I named things for #2845081: Provide a datetime_range widget to define end time via a duration offset (although @heddn RTBC'ed that, so I went forward). :)

Proposed resolution

Initial draft. Please edit! I'm trying to use the MUST, SHOULD, etc convention from coding standard docs, etc.

Field plugin IDs

{field-type}_{feature-name}

Every field-related plugin in here will operate on a specific field type. Generally either datetime or daterange. That MUST be the first part of the plugin ID.

Since the plugins already know what kind of thing they are (widget, formatter, etc) via namespace and otherwise, we don't want that also in the ID.

The _{feature-name} part tells you what feature is provided by that plugin and guarantees uniqueness. If the feature wants multiple words, separate with more underscores.

For example:

daterange_duration
daterange_compact
datetime_configurable_list

Other plugin IDs

TBD
None yet, but maybe we'll start adding other things (blocks, etc).

Config IDs

MUST start with datetime_extras. (including the period).
Otherwise, attempt to match the _{feature-name} part of the ID from any related plugins. TBD. Waves hands. ;)

Class names

Plugin classes MUST follow the agreed upon plugin ID format, plus existing core conventions.
Examples:

daterange_duration > DateRangeDurationWidget
daterange_compact > DateRangeCompactFormatter
datetime_configurable_list > DateTimeConfigurableListWidget

Services

Service IDs MUST have at least:

datetime_extras.{service-name}

Service IDs SHOULD have the form:

datetime_extras.{feature-name}.{service-name}

Example ID: datetime_extras.daterange_compact.formatter
Example class: Drupal\datetime_extras\DateRangeCompactFormatter

Remaining tasks

  1. Finish drafting the proposal, and make sure the plan covers everything we care about covering.
  2. Get agreement and sign-off from as many datetime_extras maintainers as possible:
    • @dww (here)
    • @heddn (in Slack)
    • @jhedstrom (in Slack)
    • @mpdonadio (here)
    • @pjonckiere
    • @vijaycs85
  3. Document the plan somewhere other than this issue. In CONTRIBUTING.md via #13 and #14.
  4. Unblock child issues (done) and rename whatever needs renaming (see child issues).

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

dww created an issue. See original summary.

heddn’s picture

Sounds reasonable to me. And for the mispelling, we can easily add a BC layer and fix that.

dww’s picture

Issue summary: View changes

Detailed proposal that covers a lot of these cases:
#2834016: Add 'Compact' datetime range formatter

Also, I now see @vijaycs85 is a maintainer for Date and @pjonckiere a maintainer for Calendar, so I removed the commentary by their names in the summary. I guess we don't have to get their sign-off before proceeding, but if they have thoughts, great.

Thanks,
-Derek

dww’s picture

Issue summary: View changes

Added section about naming services.

mpdonadio’s picture

I am just wondering about not namespacing plugin IDs with 'datetime_extras'. That potentially set up conflicts with being able to do things in core. But on the other hand, it make it easy to move some of these into core (if desired).

dww’s picture

I think the goal should be to make this a staging ground for things on their way into core.

Yes, there will be things that live here forever, fine.

But as we get things core-worthy, it should be easy to move things into core when ready/desired upstream.

Furthermore, if we're all closely following both worlds (as intended), then we'll be able to spot potential conflicts in advance and plan accordingly.

But I don't want to add a lot of extra characters to everything and make it harder / more clumsy to move things into core when we want to.

Thanks,
-Derek

mpdonadio’s picture

Status: Active » Reviewed & tested by the community

Been thinking about this.

I think the goal should be to make this a staging ground for things on their way into core.

+1 to this. Right now, we are doing our best to avoid do-it-all fields/widgets/formatters in core. I still agree with that, but I think this sets us up well for things that could be core worthy and force us to make good decisions along the way.

The PHP naming conventions should help with transitions to core (since just about everything is going to be a service or plugin anyway).

My only pause is that updating config via hooks is non-trivial when we move into core, https://www.drupal.org/docs/8/api/update-api/updating-configuration-in-d... The timeago one on that page wasn't fun. Doable, several examples now, but a pain.

That said, I think the IS has a pretty sane plan.

Setting RTBC. If nobody objects by the weekend, we can mark this Fixed.

dww’s picture

Issue summary: View changes

All sounds good, thanks.

Meanwhile, thoughts on point #3 from the summary:

Document the plan somewhere other than this issue?

Where might that be? ;)

Thanks again,
-Derek

mpdonadio’s picture

Maybe we can copy/paste into the README for a start? That could be the final "patch" to close out this issue.

dww’s picture

I guess that works.

Seems a bit out of scope for what most people will care about in a README.txt. Perhaps add a "DEVELOPMENT.md", instead? We can also talk about our intentions about making things "core-worthy", requiring good test coverage, etc. Again, pretty out of scope for the README crowd, but very useful for folks who want to develop / contribute.

mpdonadio’s picture

I could go for DEVELOPMENT.md or CONTRIBUTING.md. I went back and forth on README vs another file a few times.

I will post the proposal as a patch on Thursday or Friday.

dww’s picture

Sounds lovely, thanks.

mpdonadio’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2.36 KB

OK, here we go.

dww’s picture

StatusFileSize
new2.38 KB
new2.85 KB

Thanks!

  1. +++ b/CONTRIBUTING.md
    @@ -0,0 +1,70 @@
    +features that that should live in contrib.
    

    double "that"

  2. +++ b/CONTRIBUTING.md
    @@ -0,0 +1,70 @@
    +While existing as a contrib module, features to SHOULD follow Drupal best practices, such as
    

    "features to" is weird.

Also, I know it's a .md, but many folks will be reading this not as MD on the web but as a file on disk when checking out the code. So it seems we should consistently wrap to 80 chars.

Attached patch should fix everything. Interdiff is kinda big, but that's mostly due to the 80 char wrapping.

Thanks,
-Derek

  • mpdonadio committed 6c65cbc on 8.x-1.x authored by dww
    Issue #3026314 by dww, mpdonadio: Standardize naming conventions for...
mpdonadio’s picture

Status: Needs review » Fixed

Committed and pushed to 8.x-1.x.

dww’s picture

Issue summary: View changes

Great, thanks! Final cleanup of the summary to match reality.

Cheers,
-Derek

Status: Fixed » Closed (fixed)

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