Problem/Motivation

Discovered at #2972224: Add .cspell.json to automate spellchecking in Drupal core, and pointed by @xjm in https://www.drupal.org/project/drupal/issues/3122088#comment-13628724

@xjm: The word is "dependents". This is all up in our States API, so would need an issue with deprecation and all that.

Proposed resolution

1) First correct the spelling and usage of

  1. dependee
  2. dependee's
  3. dependees

2) Handle any BC concerns

Remaining tasks

Review

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#35 3138770-35.patch1.79 KBkunalgautam
#26 interdiff_20-26.txt219 bytesanmolgoyal74
#26 3138770-26.patch1.82 KBanmolgoyal74
#21 dependee.png44.54 KBshobhit_juyal
#20 3138770-19.patch1.52 KBmohrerao
#16 interdiff_11-16.txt4.19 KBdeepak goyal
#16 3138770-16.patch5.28 KBdeepak goyal
#11 interdiff_6-11.txt4.77 KBmohrerao
#11 3138770-11.patch2.43 KBmohrerao
#6 3138770-6.patch4.49 KBmohrerao

Issue fork drupal-3138770

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jungle created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Parent issue: » #3122088: [Meta] Remove spelling errors from dictionary.txt and fix them
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Title: Fix "dependents" relevant typos in core » Fix "depend" relevant typos in core
Issue summary: View changes

Adding "depending" to here, and changing the title a little bit

sja112’s picture

sja112 commented
Assigned: Unassigned » sja112
sja112’s picture

sja112 commented
Assigned: sja112 » Unassigned
mohrerao’s picture

mohrerao commented
Status: Active » Needs review
StatusFileSize
new 3138770-6.patch4.49 KB

Adding a patch for updating all comments. however i see the above words used in code as well. Not sure if we need to fix them as well at this point.

mohrerao’s picture

mohrerao commented
Issue tags: +DIACWMay2020
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Issue summary: View changes
Status: Needs review » Needs work

Thanks, @mohrerao!

  1. +++ b/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php
@@ -133,7 +133,7 @@ public function testThemeInstallWithModuleDependencies($theme_name, array $first
+    // Confirm that the dependent modules can't be uninstalled because an enabled

    Over 80 chars, enabled should move to the next line.

  2. Needs addressing "dependening" to "depending", my bad, IS updated.
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
+++ b/core/misc/states.es6.js
@@ -507,7 +507,7 @@
    * This list of states contains functions that are used to monitor the state
    * of an element. Whenever an element depends on the state of another element,
-   * one of these trigger functions is added to the dependee so that the
+   * one of these trigger functions is added to the dependent so that the
    * dependent element can be updated.

I'm not sure "dependee" is always intended to be "dependent", for example here it seems to mean the other half of the dependent relationship, ie. parent vs child.

mohrerao’s picture

mohrerao commented

Thanks for the comment @longwave. Agree #9. new patch on the way...

mohrerao’s picture

mohrerao commented
Status: Needs work » Needs review
StatusFileSize
new 3138770-11.patch2.43 KB
new interdiff_6-11.txt4.77 KB

Adding patch fixing #8.1, #8.2 and #9.

ketikagrover’s picture

ketikagrover commented
Assigned: Unassigned » ketikagrover
ketikagrover’s picture

ketikagrover commented
Assigned: ketikagrover » Unassigned
Status: Needs review » Reviewed & tested by the community

1. as per the #8 'dependening' has been addressed and changes are showing in #11
2. as per #9 changes showing in #11 and looks good to me.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Fix "depend" relevant typos in core » "Dependee" is not a word, so deprecate it in the States API in favor of "dependency"
Status: Reviewed & tested by the community » Needs work

Thanks for working on this!

"Dependee" is not a word, so even if "dependent" is not correct in context, we should say something else. I think in context the opposite of a dependent is a dependency. If "dependee" is supposed to mean the opposite of "dependent" throughout the States API, then we should use "dependency" instead.

Note that this requires somewhat complicated BC and deprecation since it involves an array key that is public API as well as JavaScript that responds to it. Therefore, I'm rescoping this issue accordingly. Note that this won't be a novice task.

Meanwhile:

+++ b/core/misc/states.es6.js
@@ -240,16 +240,16 @@
-     * Update the value of a dependee's state.
+     * Update the value of a dependees state.
...
-     *   A State object describing the dependee's updated state.
+     *   A State object describing the dependees updated state.
...
-     *   The new value for the dependee's updated state.
+     *   The new value for the dependees updated state.

This change is wrong... we still need to follow English grammar rules for the possessive. It's not "plural 'dependee'", it's "belonging to the dependee". (Or once we rename it, "dependency's" as in "belonging to the dependency".)

deepak goyal’s picture

deepak goyal commented
Assigned: Unassigned » deepak goyal
deepak goyal’s picture

deepak goyal commented
StatusFileSize
new 3138770-16.patch5.28 KB
new interdiff_11-16.txt4.19 KB

Hi @xjm
Updated patch as you suggested please review.

deepak goyal’s picture

deepak goyal commented
Status: Needs work » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I wonder if we should just leave this alone as it does seem to be in use as a programming-specific term: https://en.wiktionary.org/wiki/dependee

mohrerao’s picture

mohrerao commented

Agree with @longwave. Its better to retain dependee. But lets fix "dependee's" to "dependees" and close.

mohrerao’s picture

mohrerao commented
Assigned: deepak goyal » Unassigned
StatusFileSize
new 3138770-19.patch1.52 KB

Adding a patch for replacing dependee's to dependees.

shobhit_juyal’s picture

shobhit_juyal commented
StatusFileSize
new dependee.png44.54 KB

Not sure what is finalized at the end as per the xjm comment at #14. It may need to just have a look on this.

modules/system/tests/src/Functional/Theme/ThemeUiTest.php:    // Enable the first group of dependee modules.
modules/system/tests/src/Functional/Theme/ThemeUiTest.php:    // Enable the second group of dependee modules.

Not changing the status for now.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Needs review » Needs work
Issue tags: +Global2020, +Novice

Let's continue based on comment #18

  1. Typos should be removed from the dictionary file core/misc/cspell/dictionary.txt
  2. Tagging Global2020, and Novice
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I think "dependee's" is correct as well, it is the possessive of "dependee".

mrinalini9’s picture

mrinalini9 commented

Can someone please update IS which words needs to be replaced or retained?

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.

anmolgoyal74’s picture

anmolgoyal74 commented
Status: Needs work » Needs review
StatusFileSize
new 3138770-26.patch1.82 KB
new interdiff_20-26.txt219 bytes

Removed dependee's from core/misc/cspell/dictionary.txt

anmolgoyal74’s picture

anmolgoyal74 commented
Issue summary: View changes
mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs review » Needs work
Issue tags: +Europe2020, +Needs issue summary update

I performed Novice Triage on this issue. I am leaving the Novice tag on this issue because I think there is still clear action to do including the excellent suggestion by @mrinalini9 that the issue summary and issue title needs updating. Specifically the proposed resolution has changed from @xjm's comments based on @longwave and @mohrerao .

However, if dependee is a technical, programming term, I think that it at least should be defined as @xjm mentioned. It would help make what dependee is clearer if that property were described/defined as a dependency in code block for states.Dependent (the constructor).

I changed the status to Needs work based on the review below.

+++ b/core/misc/cspell/dictionary.txt
--- a/core/misc/states.es6.js
+++ b/core/misc/states.es6.js

The es6 version needs to be transpiled to a es5 version in the patch as well.

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.

sudiptadas19’s picture

sudiptadas19 commented
Status: Needs work » Needs review

@mradcliffe
Trying to address your suggestions given in #28, the changes made in #26 are related to documentation changes so I don't think the transpiled copy is required for this.
I have run the below-given code but there are no changes after the successful command execution.
yarn run build:js -- --file misc/states.es6.js
Please suggest.

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.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Was tagged for IS update and that appears to still need to happen.

kunalgautam’s picture

kunalgautam commented
StatusFileSize
new 3138770-35.patch1.79 KB

Patch Drupal version 10.1.x

kunalgautam’s picture

kunalgautam commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Please read the tags also this still needs an issue summary update

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 made their first commit to this issue’s fork.

quietone’s picture

quietone commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Novice, -Needs issue summary update

This really needs someone who know the State API to confirm the word changes.

quietone opened merge request !6535

quietone’s picture

quietone commented
Issue tags: +cspell error
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

So tried to find someone, thought maybe we had a states API manager but do not.

Looking at the pure replacement used though and it appears to be a good 1-1 update.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

The issue summary and title say the opposite of what the patch does, could use an update.

smustgrave’s picture

smustgrave commented
Title: "Dependee" is not a word, so deprecate it in the States API in favor of "dependency" » Replace incorrect words for Dependee in States API
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs issue summary update

Updated issue summary.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs review

What about the concerns raised in #14?

dww’s picture

dww
we/he/they
commented

And what about the counter-proposal in #18? Maybe it's better to leave this alone, per https://en.wiktionary.org/wiki/dependee -- "Dependee" is enough of a "word" to have a wikipedia article devoted to itself. Perhaps we should call this "works as designed" and move on?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Some discussion on this word at https://english.stackexchange.com/questions/25575/what-is-the-correct-wo...

nod_’s picture

nod_
Primary language French
Location Lille
commented

#46 is correct, the BC layer is missing.

I'm not native and dependee makes sense to me (like employer/employee) so to take with a grain of salt: I'd be for leaving this alone given the work needed to make it work properly (depreciation, bc layer, etc.)

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Needs work
smustgrave’s picture

smustgrave commented

So is the new consensus to leave as is?

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Closed (works as designed)

yes, consulted with other core committers we're moving the word to drupal-dictionary.

see #3422584: Keep the word dependee and move it to drupal-dictionary.txt

catch’s picture

catch
he/him
Primary language English
commented

I think a similar example is where performant got added to the Cambridge dictionary due to its technical usage even though it was not really an English word before that, although in this case nod_ pointed out it's also a real French word that translates to the same meaning as its used in English. So I think we can treat it as a neologism rather than a mis-spelling. I think if we were going to try to change this in the states API we might want to completely change the vocabulary (no idea what to, but something without the ambiguity of dependency/dependent/dependee).