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
- Finish drafting the proposal, and make sure the plan covers everything we care about covering.
- 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
Document the plan somewhere other than this issue.In CONTRIBUTING.md via #13 and #14.Unblock child issues(done) and rename whatever needs renaming (see child issues).
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3026314.13_14.interdiff.txt | 2.85 KB | dww |
| #14 | 3026314-14.patch | 2.38 KB | dww |
| #13 | 3026314-13.patch | 2.36 KB | mpdonadio |
Comments
Comment #2
heddnSounds reasonable to me. And for the mispelling, we can easily add a BC layer and fix that.
Comment #3
dwwDetailed 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
Comment #4
dwwAdded section about naming services.
Comment #5
mpdonadioI 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).
Comment #6
dwwI 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
Comment #7
mpdonadioBeen thinking about this.
+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.
Comment #8
dwwAll sounds good, thanks.
Meanwhile, thoughts on point #3 from the summary:
Where might that be? ;)
Thanks again,
-Derek
Comment #9
mpdonadioMaybe we can copy/paste into the README for a start? That could be the final "patch" to close out this issue.
Comment #10
dwwI 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.
Comment #11
mpdonadioI 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.
Comment #12
dwwSounds lovely, thanks.
Comment #13
mpdonadioOK, here we go.
Comment #14
dwwThanks!
double "that"
"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
Comment #16
mpdonadioCommitted and pushed to 8.x-1.x.
Comment #17
dwwGreat, thanks! Final cleanup of the summary to match reality.
Cheers,
-Derek