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.
Follow up for #2017985: ConfigTranslationManageForm.php standards cleanup to get ready for getting into core
Updated: Comment #0
Problem/Motivation
We sometimes use the short word config instead of the english word configuration in our docs.
Proposed resolution
config -> configuration in all our docs
Remaining tasks
See if there are places we might want to keep config. Suspect, those would only be for actual class names, Config.
User interface changes
No.
API changes
No.
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#14 | config_translation_2038743_14.patch | 14.21 KB | YesCT |
#14 | interdiff-13-14.txt | 1.34 KB | YesCT |
#13 | config_translation_2038743_13.patch | 15.2 KB | YesCT |
#13 | interdiff-11-13.txt | 9.25 KB | YesCT |
#11 | config_translation_2038743_11.patch | 21.77 KB | YesCT |
Comments
Comment #1
LinL CreditAttribution: LinL commentedHere's a patch for this. The only one I (intentionally!) left is in this todo:
in ConfigTranslationDeleteForm.php
Comment #2
YesCT CreditAttribution: YesCT commentedComment #3
Xano@LinL Apparently we started working on this simultaneously :)
My patch seems to do exactly what #1 does, but it also replaces some internal variable names. It does not touch any API-related code.
Comment #4
LinL CreditAttribution: LinL commented@Xano :) Good that they agree!
I noticed this one extra change I had in #1:
Comment #5
YesCT CreditAttribution: YesCT commentedWhy should we even invest in this? Is it worth it?
In part,
I might "config" something. that's configure.
I might check the "config". that's configuration.
If someone tries to put comments into a translation tool, like google translate, to understand better what the comments mean, will it translate "config" right?
I tried some comment blocks, and translating to spanish, it seems to understand that config is configuration.
=====
So a Config object is really just: Config object, Config is the name of the class.
But it's a configuration object, describing what that is in words.
------
Should we add articles like "a" and "the" while we are changing these lines?
------------
actually, @todo's have additional lines indented 2 spaces. https://drupal.org/node/1354#todo
-------------
tricky thing words and names of things.
Take this one.
Let's keep it ConfigGroupMapper and not...
ConfigurationGroupMapper implements ConfigurationMapperInterface
public function getConfigurationGroup
And changing it from
This is already a config group.
to
This is already a configuration group.
is a good change.
The alternative would be instead:
This is already a ConfigGroupMapper.
--- aside ---
Also, why is it a "group"?
Group is not used to describe this ConfigGroupMapper. Maybe that should be Mapper for the editing screen of a group of configuration.
--- end aside ---
Here a couple examples of code using config vs configuration.
In this case, getConfigData is returning an array of configuration information, and not a specific Config class, so I think $base_configuration makes sense.
------
I didn't look at every change in the patch.
Comment #6
YesCT CreditAttribution: YesCT commentedcore is already inconsistant in how it does this.
Comment #7
XanoComment #8
YesCT CreditAttribution: YesCT commentedYeah, the trouble with long words... makes one line summaries longer than one line.
We should shorten this line. For the one line summary, if needed can add a blank line and then a paragraph for more explanation to make sure we retain all the info.
I just reworded it.
this needs to wrap at 80 chars too.
---------
I read the whole patch.
I rechecked all the files for short config in the comments and did not see any.
I dont feel strongly about the $configuration variable names, so I'm fine with it like this.
We dont want to drive our selves crazy, so I think we are ok this.
Comment #9
Gábor HojtsyI think renaming the variables is a mistake. The core function we get configuration from is config(). The namespace is Drupal\Core\Config\Config, the module name is config_translation, and so on and on. I think config is well understood in this sense. I see that the comments are cleaner to spell out the full word, but the API does not do that, so I'd not rename variables on this either. That would also make this patch *much* easier to review.
Comment #10
YesCT CreditAttribution: YesCT commentedok. I'll do it.
Comment #11
YesCT CreditAttribution: YesCT commentedhere is just a reroll.
Comment #13
YesCT CreditAttribution: YesCT commentednot doing the variable renames.
unrelated failure should be fixed by #2054183: shortcut renamed
Comment #14
YesCT CreditAttribution: YesCT commentedoops. missed a set.
Comment #16
YesCT CreditAttribution: YesCT commented#14: config_translation_2038743_14.patch queued for re-testing.
Comment #17
Gábor HojtsyYay, thanks, looks good to me, committed!