From https://www.drupal.org/project/drupal/issues/3111409#comment-13841374

JS documentation and comments need to adhere to https://www.drupal.org/docs/develop/standards/javascript/javascript-api-...

CommentFileSizeAuthor
#50 3173832-50.patch11.87 KBpaulocs
#50 interdiff-47-50.txt2.87 KBpaulocs
#47 interdiff-46-47.txt4.08 KBmherchel
#47 3173832-47.patch11.18 KBmherchel
#46 3173832-46-reroll.patch10.07 KBmherchel
#45 core-oliverojs-3173832-45.patch10.47 KBSakthivel M
#42 core-oliverojs-3173832-42.patch10.41 KBnod_
#42 interdiff-38-42.txt4.05 KBnod_
#38 interdiff-37_38.patch1.22 KBGauravvvv
#38 3173832-38.patch6.87 KBGauravvvv
#37 3173832-37.patch6.88 KBpaulocs
#36 3173832.36.patch6.91 KBSakthivel M
#32 diff_30-32.txt9.66 KBkostyashupenko
#32 3173832-32.patch9.45 KBkostyashupenko
#31 patch-apply-issues.png45.4 KBsulfikar_s
#30 3173832-30.patch15.99 KBmherchel
#30 interdiff-29-30.txt5.63 KBmherchel
#29 interdiff_26-29.txt3.01 KBadamzimmermann
#29 3173832-29.patch8.08 KBadamzimmermann
#26 interdiff_19-26.txt5.72 KBadamzimmermann
#26 3173832-26.patch8.31 KBadamzimmermann
#24 interdiff_19_23.txt1.33 KBKapilV
#23 3173832-23.patch3.03 KBKapilV
#22 3173832-22.patch8.05 KBKapilV
#19 interdiff-3173832-16-19.txt32.85 KBmarkdorison
#19 3173832-19.patch8.34 KBmarkdorison
#16 3173832-16.patch36.91 KBPooja Ganjage
#10 3173832-10.patch7.34 KBPooja Ganjage
#7 3173832-7.patch5.38 KBPooja Ganjage
#5 3173832-5.patch15.02 KBPooja Ganjage
#4 3173832-4.patch15.22 KBPooja Ganjage
#2 3173832-2.patch15.18 KBPooja Ganjage

Issue fork drupal-3173832

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mherchel created an issue. See original summary.

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3173832-2.patch15.18 KB

Hi,

I am creating patch for the issue.

Please review the patch.

Let me know if any recommendations.

Thanks.

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Active » Needs review
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3173832-4.patch15.22 KB
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3173832-5.patch15.02 KB
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Needs work

Thanks for the work! There's a number of issues with the patch. We need to fix indentation issues, and be more descriptive on almost all of the comments. 

  1. +++ b/js/comments.es6.js
@@ -1,9 +1,15 @@
+ * Theme for comments.

    All of these comments are very ambiguous. for each of these, we need to document what the file or function does. "Theme for comments" doesn't accomplish this. 

  2. +++ b/js/debug.es6.js
@@ -1,4 +1,17 @@
+/**

    Debug.es6.js is being removed from the core patch. We don't need to modify this. 

  3. +++ b/js/messages.es6.js
@@ -1,7 +1,10 @@
+ * Attaches the table drag behavior to tables.

    I'm confused. We're not attaching table drag. 

  4. +++ b/js/navigation.es6.js
@@ -1,3 +1,8 @@
+ * Theme for navigation.

    Needs to be more descriptive. 

  5. +++ b/js/polyfills.es6.js
@@ -1,4 +1,6 @@
+/**

    I don't think this is valid for eslint. 

  6. +++ b/js/scripts.es6.js
@@ -6,6 +11,11 @@
+   * A isDesktopNav function.

    Needs to be more descriptive. 

  7. +++ b/js/scripts.es6.js
@@ -19,10 +29,20 @@
+   * A wideNavIsOpen function.

    Needs to be more descriptive. 

  8. +++ b/js/scripts.es6.js
@@ -19,10 +29,20 @@
+   * Provides the function for wideNavIsOpen.

    Needs to be more descriptive. 

  9. +++ b/js/scripts.es6.js
@@ -19,10 +29,20 @@
+   * A showWideNav function.

    Needs to be more descriptive. 

  10. +++ b/js/scripts.es6.js
@@ -19,10 +29,20 @@
+   * Provides the function for showWideNav.

    Needs to be more descriptive. 

  11. +++ b/js/scripts.es6.js
@@ -30,7 +50,11 @@
+   * A hideWideNav function.

    Needs to be more descriptive. 

  12. +++ b/js/scripts.es6.js
@@ -47,84 +74,100 @@
+   * A toggleDesktopNavVisibility function.

    Needs to be more descriptive. 

  13. +++ b/js/scripts.es6.js
@@ -47,84 +74,100 @@
+   *   Script for entries.

    Needs to be more descriptive. 

  14. +++ b/js/scripts.es6.js
@@ -47,84 +74,100 @@
+   * Provides the function for getRootMargin.

    Needs to be more descriptive. 

  15. +++ b/js/scripts.es6.js
@@ -47,84 +74,100 @@
+   * A monitorNavPosition function.

    Needs to be more descriptive. 

  16. +++ b/js/scripts.es6.js
@@ -47,84 +74,100 @@
+   * Provides the function for monitorNavPosition.

    Needs to be more descriptive. 

  17. +++ b/js/scripts.es6.js
@@ -47,84 +74,100 @@
+  if (e.keyCode === 27) {

    Why the change in indentation? 

  18. +++ b/js/search.es6.js
@@ -1,12 +1,27 @@
+ * Theme for search.

    Needs to be more descriptive. 

  19. +++ b/js/search.es6.js
@@ -1,12 +1,27 @@
+   * Provides the function for searchIsVisible.

    Needs to be more descriptive. 

  20. +++ b/js/search.es6.js
@@ -1,12 +1,27 @@
+   * A handleFocus function.

    Needs to be more descriptive. 

  21. +++ b/js/search.es6.js
@@ -15,6 +30,12 @@
+   * A toggleSearchVisibility function.

    Needs to be more descriptive. 

  22. +++ b/js/second-level-navigation.es6.js
@@ -1,3 +1,8 @@
+ * Theme for second-level-navigation.

    Needs to be more descriptive. 

  23. +++ b/js/tabs.es6.js
@@ -1,35 +1,63 @@
+   * @param {object} el

    Needs to be more descriptive. 

  24. +++ b/js/tabs.es6.js
@@ -1,35 +1,63 @@
+  /**

    Why the change in indentation? 

  25. +++ b/js/tabs.es6.js
@@ -1,35 +1,63 @@
+   *   Tabs for e.

    e is an event object. 

  26. +++ b/js/tabs.es6.js
@@ -1,35 +1,63 @@
+  function handleTriggerClick(e) {

    Why change of indentation? 

  27. +++ b/js/tabs.es6.js
@@ -1,35 +1,63 @@
+   * Attaches the tabs.

    Needs to be more descriptive.

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3173832-7.patch5.38 KB
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Needs work » Needs review
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Smile commented
Status: Needs review » Needs work

Latest patch adds tabs, and dosen't apply cleanly on my end

For the behaviors comments, there needs to be a little more precision, mainly the @type of the behavior.xxx object, and the type of the attach function, like this: 

  /**
   * <Main description>.
   *
   * @type {Drupal~behavior}
   *
   * @prop {Drupal~behaviorAttach} attach
   *   <Attach method description>.
   */
  Drupal.behaviors.xxx = { … };
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3173832-10.patch7.34 KB
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Needs work » Needs review
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and at Smile commented
Status: Needs review » Needs work

Patch still doesn't apply, comments for behaviors are not using the core standards. See #9

can you make sure git apply your-patch.patch works before sending a new patch? thanks. Latest patch fail to apply on: 

olivero$ git apply ../3173832-10.patch
error: le patch a échoué : js/comments.es6.js:1
error: le patch a échoué : js/messages.es6.js:1
error: le patch a échoué : js/navigation.es6.js:1
error: le patch a échoué : js/scripts.es6.js:1
error: le patch a échoué : js/search.es6.js:3
error: le patch a échoué : js/second-level-navigation.es6.js:1
error: le patch a échoué : js/tabs.es6.js:1
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Title: Ensure JS documentation matches standards » Ensure Olivero's JS documentation matches standards
Project: Olivero » Drupal core
Version: 8.x-1.x-dev » 9.1.x-dev
Component: Code » Olivero theme
Priority: Normal » Minor
ravi.shankar’s picture

ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commented
Assigned: Unassigned » ravi.shankar
ravi.shankar’s picture

ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commented
Assigned: ravi.shankar » Unassigned
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3173832-16.patch36.91 KB

Hi,

Providing updated patch.

Please review the patch.

Thanks.

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Needs work » Needs review
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Needs work

There's a lot of unrelated changes besides comment in this patch. Please only modify the comments. Also, please test your patches before submitting them.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs work » Needs review
FileSize
3173832-19.patch8.34 KB
interdiff-3173832-16-19.txt32.85 KB

Re-rolled the patch to apply cleanly and made my best effort to remove non-comment-related changes as mentioned in #18

I am guessing this will still need work but marking as needs review so it will be tested.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Needs work

There's also a lot of improper grammar in the previous patch. And there are a number of things that do not make sense in context such as "Monitoring navigation position as per using root margin."

Honestly the whole patch needs work to be useful to future devs. We need to be sure the comments make sense, and use proper grammar.

KapilV’s picture

KapilV CreditAttribution: KapilV as a volunteer and commented
Assigned: Unassigned » KapilV
KapilV’s picture

KapilV CreditAttribution: KapilV as a volunteer and at Innoraft for Drupal Association commented
Assigned: KapilV » Unassigned
Status: Needs work » Needs review
FileSize
3173832-22.patch8.05 KB
KapilV’s picture

KapilV CreditAttribution: KapilV as a volunteer and at Innoraft for Drupal Association commented
FileSize
3173832-23.patch3.03 KB
KapilV’s picture

KapilV CreditAttribution: KapilV as a volunteer and at Innoraft for Drupal Association commented
FileSize
interdiff_19_23.txt1.33 KB
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Needs work

There's still a lot of bad grammar within the comments. We need to use proper grammar, and also be sure to not remove the end of file line break.

adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Status: Needs work » Needs review
FileSize
3173832-26.patch8.31 KB
interdiff_19-26.txt5.72 KB

I'm not a front-end developer, so I took some educated guesses at what some of the code was doing. I'm sure someone better versed in the code could add more detailed descriptions, but I'm not sure that is needed. What that in mind, I simplified some of the method descriptions to focus more on what/why vs the how.

Patch #23 seemed to be reverting some changes we wanted, so I started from #19. Happy to reincorporate any wanted changes though.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Needs work

Thanks for the work on this. I'm attaching some comments!

  1. +++ b/core/themes/olivero/js/comments.es6.js
@@ -1,9 +1,15 @@
+   * @return {boolean}

    The function doesn't return anything. It adds the show/hide functionality to the comments. 

  2. +++ b/core/themes/olivero/js/scripts.es6.js
@@ -1,7 +1,12 @@
+ * Controls the visibility of navigation for different layouts.

    This doesn't change based on layouts. It shows and hides the desktop nav based on scroll position and controls the functionality of the button that shows/hides the nav. 

  3. +++ b/core/themes/olivero/js/search.es6.js
@@ -2,11 +2,20 @@
+   *   True if navWrapper contains "is-active" class, false if not.

    It might be good to elaborate on what visibility is (it is the state where we want the search form to go to) 

  4. +++ b/core/themes/olivero/js/tabs.es6.js
@@ -1,13 +1,36 @@
+   *   Tabs for el.

    This should be elaborated on. 

  5. +++ b/core/themes/olivero/js/tabs.es6.js
@@ -1,13 +1,36 @@
+     *   Check value is greater then 0 for trigger tabs.

    This comment doesn't make sense. It returns false if the mobile tabs trigger button isn't visible, and true if it is. 

  6. +++ b/core/themes/olivero/js/tabs.es6.js
@@ -1,13 +1,36 @@
+     *   Using object set the attribute value as "true" or "false".

    This is an event object 

  7. +++ b/core/themes/olivero/js/tabs.es6.js
@@ -30,6 +53,14 @@
+   * @prop {object} attach

    Do we need this?

adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Assigned: Unassigned » adamzimmermann

I'll take another pass at this today.

adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Assigned: adamzimmermann » Unassigned
Status: Needs work » Needs review
FileSize
3173832-29.patch8.08 KB
interdiff_26-29.txt3.01 KB
  1. Removed.
  2. Updated. Thanks for the explanation.
  3. I'm not sure I understand your request here. This method isn't about wanting the state to go anywhere, it's just reporting on what it finds.
  4. Updated.
  5. Updated.
  6. Updated.
  7. Removed.
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Version: 9.1.x-dev » 9.2.x-dev
FileSize
interdiff-29-30.txt5.63 KB
3173832-30.patch15.99 KB

Thanks! I went through and made some changes to clear things up.

sulfikar_s’s picture

sulfikar_s CreditAttribution: sulfikar_s at Zyxware Technologies commented
Status: Needs review » Needs work
FileSize
patch-apply-issues.png45.4 KB

Hi, I've applied patch on #30 and it gets applied, but it shows some errors like below.

patch-apply-issues.png

Also found that Patch includes a modification to the interdiff file (interdiff-29-30.txt) !!!

Please work on this.

kostyashupenko’s picture

kostyashupenko
Primary language Russian
Location Omsk
CreditAttribution: kostyashupenko commented
FileSize
3173832-32.patch9.45 KB
diff_30-32.txt9.66 KB

Rerolled + excluded interdiff-29-30.txt from patch file
Also added 2 strings to pass lint:core-js script ==>>>

scripts.es6.js
...
*   True if navButtons is hidden, false if not.
...
*   True if "aria-expanded" attribute of the wideNavButton contains "true" value, false if not.
...
kostyashupenko’s picture

kostyashupenko
Primary language Russian
Location Omsk
CreditAttribution: kostyashupenko commented
Status: Needs work » Needs review

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.

andy-blum’s picture

andy-blum
he/him/his
Primary language English
Location Ohio, USA
CreditAttribution: andy-blum at Lullabot commented
Status: Needs review » Needs work

Needs a re-roll

Sakthivel M’s picture

Sakthivel M CreditAttribution: Sakthivel M at QED42 for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3173832.36.patch6.91 KB

#32 Patch Failed,

#36 Recreated patch, Please review the patch

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
FileSize
3173832-37.patch6.88 KB

Adding new patch, because #36 failed to apply.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at OpenSense Labs commented
FileSize
3173832-38.patch6.87 KB
interdiff-37_38.patch1.22 KB

Please review.

I mistyped interdiff as .patch file. Ignore that

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Status: Needs review » Reviewed & tested by the community
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/themes/olivero/js/scripts.es6.js
@@ -1,11 +1,20 @@

    Extra space here. 

  2. +++ b/core/themes/olivero/js/scripts.es6.js
@@ -91,7 +100,7 @@
+  // Only enable scroll interactivity if the browser supports Intersection Observer.

    Nit: line over 80 characters. 

  3. +++ b/core/themes/olivero/js/scripts.es6.js
@@ -106,7 +115,7 @@
+        // Firefox doesn't seem to support entry.isIntersecting properly,

    Nit: we can remove "seem to" from here. 

  4. +++ b/core/themes/olivero/js/scripts.es6.js
@@ -173,6 +188,9 @@
+    /**
+     * Monitor the navigation position.
+     */

    This comment is probably redundant because next line from here makes it pretty obvious that this sets up monitoring for nav position.

  5. Let's document behaviors using the example here https://www.drupal.org/docs/develop/standards/javascript/javascript-api-... 
  6. +++ b/core/themes/olivero/js/navigation.es6.js
@@ -1,3 +1,7 @@
    * Checks if navWrapper contains "is-active" class.

    After the first line there should be empty line. There are many docblocks in this file with this violation.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and commented

saw a few other things, patch comming.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and at Acquia commented
Status: Needs work » Needs review
FileSize
interdiff-38-42.txt4.05 KB
core-oliverojs-3173832-42.patch10.41 KB

I thought we had some eslint rules to check documentation format. in any case spotted a few more things.

missed #3 from lauriii post, todo

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Reviewed & tested by the community

#42 looks perfect. Thanks everyone!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs work

Sorry, here's few more nitpicks 😅 Probably could have fixed on commit but I wasn't sure about .1 and .3.

  1. +++ b/core/themes/olivero/js/second-level-navigation.es6.js
@@ -60,7 +67,8 @@
+   *   event object

    This should be a sentence and end in a full stop. 

  2. +++ b/core/themes/olivero/js/navigation.es6.js
@@ -45,7 +54,8 @@
+    // Close any open sub-navigation first, then close the  header navigation.

    Nit: double space between "the" and "header". 

  3. +++ b/core/themes/olivero/js/navigation.es6.js
@@ -45,7 +54,8 @@
+    // slide-out.

    What does the slide-out mean here? Could we make it into a full sentence?

Sakthivel M’s picture

Sakthivel M CreditAttribution: Sakthivel M at QED42 for Drupal India Association commented
Status: Needs work » Needs review
FileSize
core-oliverojs-3173832-45.patch10.47 KB

Providing updated version of patch #45 as per comment #44, Please review the patch

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
FileSize
3173832-46-reroll.patch10.07 KB

@Sakthivel M Thanks for the patch, but next time please provide an interdiff so we can see exactly whats changed.

Re-roll of 42 attached.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
FileSize
3173832-47.patch11.18 KB
interdiff-46-47.txt4.08 KB

Made a number of little nit-picky changes (see interdiff) including re-doing those from #44/#45

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Status: Needs review » Needs work

This is looking really good. I spotted a few other little nits:

  1. +++ b/core/themes/olivero/js/checkbox.es6.js
@@ -1,6 +1,6 @@
+ * Creates a checkbox input element.

    Something a little more generic here means you won't have to rewrite it if additional checkbox related functionality is added in this file. 

  2. +++ b/core/themes/olivero/js/checkbox.es6.js
--- a/core/themes/olivero/js/comments.es6.js
+++ b/core/themes/olivero/js/comments.es6.js

    The @file annotation could benefit from being more generalized so it doesn't need to change if the file adds functionality not specific to show/hide of nested comments. 

  3. +++ b/core/themes/olivero/js/comments.es6.js
@@ -5,10 +5,10 @@
+   * Initialize the show/hide button for the comments.

    An unnecessary "the" here. 

  4. +++ b/core/themes/olivero/js/comments.es6.js
--- a/core/themes/olivero/js/messages.es6.js
+++ b/core/themes/olivero/js/messages.es6.js

    The closeMessage function doc could use an "a" after "Adds". 

  5. +++ b/core/themes/olivero/js/messages.es6.js
@@ -1,6 +1,6 @@
+ * Adds a close button to messages.

    This describes one function within this file, not the contents of the overall file. 

  6. +++ b/core/themes/olivero/js/messages.es6.js
@@ -108,7 +108,7 @@
-   * Getting messages from context.
+   * Get messages from context.
    */
   Drupal.behaviors.messages = {
     attach(context) {

    This needs the whole 

       * @type {Drupal~behavior}
   *
   * @prop {Drupal~behaviorAttach} attach

    thing. 

  7. +++ b/core/themes/olivero/js/navigation.es6.js
@@ -1,8 +1,15 @@
    * @param {object} navWrapper

    {Element}? 

  8. +++ b/core/themes/olivero/js/navigation.es6.js
index 8bea1fb96b..79432afc06 100644
--- a/core/themes/olivero/js/scripts.es6.js

    function stickyHeaderIsEnabled() Needs docs 

  9. +++ b/core/themes/olivero/js/search.es6.js
@@ -1,3 +1,8 @@
+ * Provides interactivity for showing and hiding the search.

    Similar to other files, could be good to make this more generic so it remains accurate even if the functionality provided in this file expands.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Assigned: Unassigned » paulocs
paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Assigned: paulocs » Unassigned
Status: Needs work » Needs review
FileSize
interdiff-47-50.txt2.87 KB
3173832-50.patch11.87 KB

I addressed what @bnjmnm pointed in #48.
I tried to write a more generic file documentation but I'm not sure if it looks good.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Reviewed & tested by the community

#50 looks great. Thanks for working on the patch!

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and at Acquia commented

Looking good, RTBC+1

  • lauriii committed f0a97d0 on 9.3.x 
    Issue #3173832 by Pooja Ganjage, mherchel, adamzimmermann, paulocs,...

  • lauriii committed 2b2f312 on 9.2.x 
    Issue #3173832 by Pooja Ganjage, mherchel, adamzimmermann, paulocs,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Opened #3222313: Rename scripts.js to something more descriptive as a follow-up.

Committed f0a97d0 and pushed to 9.3.x. Also cherry-picked to 9.2.x because this changes documentation only, and Olivero is experimental. Thanks!

Status: Fixed » Closed (fixed)

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