- 🇮🇳India vinmayiswamy
In Drupal 11.x., there are 104 mentions of folder/folders in 50 core files (excluding variable names). Is it better to create a mega MR with all the changes at once or with smaller MRs with a batch of files’ changes?
- 🇳🇿New Zealand quietone
@VinmayiSwamy, that is a good question, thanks for asking! Drupal does have a href=" https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... → ">issue scope scope guidelines. Reading that should help to get started on this.
I searched core for 'folder' and see some that are only comments with no associated code. Those are probably easy ones to change. I didn't spot any that are in user facing strings but I could have missed them. If there are any, they would be a candidate for a separate child issue.
Then there is InstallStorage which uses both the word 'directory' and 'folder'. That would need investigation to understand why both are used. And that file has a class property $folders and a class property $directory. It is certainly not a simple change here. The class also has a method getAllFolders(). Changing the method name and class properties requires a deprecation.
We also need to consider how to prevent the use of 'folders' in the future.
- 🇮🇳India vinmayiswamy
@quietone, thank you for your detailed response and guidance. I’ve read the issue scope guidelines and it has provided me with a clearer understanding of how to proceed.
I agree with your suggestion to separate the changes into different categories. For the instances of ‘folder’ that are only in comments with no associated code, I will start by creating a MR to address these.
For the user-facing strings, if any, I will investigate further and if necessary, create a separate child issue as you suggested.
Regarding the
InstallStorage
class, I understand the complexity due to the use of both ‘directory’ and ‘folder’, as well as the presence of the class property$folders
,$directory
, and the methodgetAllFolders()
. I will conduct a thorough investigation to understand why both terms are used and plan the changes accordingly, keeping in mind the need for deprecation for changing method names and class properties.Lastly, to prevent the future use of ‘folders’, I believe updating the Drupal coding standards documentation and educating contributors could be effective. I’m open to suggestions on this.
Thank you again for your guidance. I will proceed with these steps and keep the issue updated.
- Assigned to vinmayiswamy
- Merge request !83133151207 - relplace references to folder with directory → (Open) created by vinmayiswamy
- 🇮🇳India vinmayiswamy
@quietone, as previously discussed, I have created a MR (merge request !8313) to address the instances of ‘folder’ and ‘folders’ that are only in comments with no associated code. Kindly please review this MR.
For the next steps, I have identified 149 mentions in 22 files of “folder” or “folders” in the core that are associated with code. I will be investigating these instances further to understand the context and determine the best course of action.
One area that requires special attention is the InstallStorage class, which uses both ‘directory’ and ‘folder’, as well as the presence of the class property
$folders
,$directory
, and the methodgetAllFolders()
. I will conduct a thorough investigation to understand why both terms are used and plan the changes accordingly, keeping in mind the need for deprecation for changing method names and class properties.Thanks!
- Issue was unassigned.
- 🇮🇳India vinmayiswamy
Leaving this issue unassigned, as mentioned in the Issue etiquette →
It says we should avoid assigning tickets to ourselves unless you're a maintainer.Thanks!