Re-organise devtools/client/webconsole/components/ folder

RESOLVED FIXED in Firefox 69

Status

task
P2
normal
RESOLVED FIXED
7 months ago
2 months ago

People

(Reporter: nchevobbe, Assigned: armando.ferreira, Mentored)

Tracking

({good-first-bug})

Trunk
Firefox 69
Points:
---

Firefox Tracking Flags

(firefox69 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

As we're adding new components, the number of file is growing and the components folder is hard to navigate in.

We could add additional folder to reflect the component hierarchy:

▼ components
| - App.js
| - Sidebar.js
| ▼ FilterBar
| | - FilterBar.js
| | - FilterButton.js
| | - FilterCheckbox.js
| ▼ Input
| | - ConfirmDialog.js
| | - EditorToolbar.js (not existing yet)
| | - JsTerm.js
| | - ReverseSearchInput.js
| ▼ Output
| | - ConsoleOutput.js
| | …+ all Message related component

Mentor: nchevobbe
Keywords: good-first-bug
Summary: Re-organise devtools components folder → Re-organise devtools/client/webconsole/components/ folder

Can I take this up?

Hello Srestha!
Sure, thanks for wanting to help us.

If you don't have a work environment yet, I suggest you to read https://docs.firefox-dev.tools/getting-started/
When asked, please be sure to use "Artifact builds", as it speeds up development time a lot.

Then, you should be able to work on this bug.
You will need to:

  • Create 3 folders in devtools/client/webconsole/components/ (FilterBar, Input, Output)
  • Add them to the DIR array in devtools/client/webconsole/components/moz.build
  • Move the files to the relevant folder (using hg mv devtools/client/webconsole/components/FilterBar.js devtools/client/webconsole/components/FilterBar/FilterBar.js for example). This is important as we need to keep the history on the files.
  • On each new folder, create a moz.build file, and reference all the files in the folder (like in devtools/client/webconsole/components/moz.build)
  • In devtools/client/webconsole/components/moz.build, remove the files that were moved.
  • And then the trickiest part: in the javascript code, fix all the paths used to require the modules. For example the following:
const FilterBar = createFactory(require("devtools/client/webconsole/components/FilterBar"));

should be transformed into

const FilterBar = createFactory(require("devtools/client/webconsole/components/FilterBar/FilterBar"));

and so on.

I encourage you to use https://searchfox.org/ to search for the files you moved an see where they're referenced. Then, running firefox and try to open the console should tell you if everything was moved correctly or not :)

Don't hesitate to ask questions here, or on our Slack

Assignee: nobody → sresthasrivastava.ss
Status: NEW → ASSIGNED

In which folder should I put CollapseButton.js and ConsoleTable.js?

(In reply to SresthaSrivastava [:srestha] from comment #3)

In which folder should I put CollapseButton.js and ConsoleTable.js?

In the output folder (these components are used from Message)

Should I put ReverseSearchInput.css also in Input folder?

Please review

Attachment #9040167 - Attachment description: Bug 1522104 - Re-organised folders. → Bug 1522104 - Re-organised devtools/client/webconsole/components r=nchevobbe

Hello srestha!
Is there anything blocking you on this bug?
No worries if you simply don't have the time to look into this :)

Flags: needinfo?(sresthasrivastava.ss)

Hey, sorry I got busy with something, I'll just get it done in a day or two.

Flags: needinfo?(sresthasrivastava.ss)
Posted file gh (obsolete) —
Attachment #9046832 - Attachment description: Bug 1522104 Reorganised folders → Bug 1522104 Reorganised folders r=nchevobbe

Ignore this latest attachment^.
Please review the first attachment.

Hello Srestha, there's 2 attachments and I'm confused which one I should have a look at?
Which is the one that you rebased? Also, can you close the other one (Abandon changes in phabricator) please? Thanks!

Attachment #9046832 - Attachment description: Bug 1522104 Reorganised folders r=nchevobbe → gh

How do I abandon changes in phabricator?

Thanks Srestha, will do in the next hour.

To abandon the revision, you need to go to https://phabricator.services.mozilla.com/D21264, and at the very bottom, there is a dropdown with an abandon the revision item.

Attachment #9046832 - Attachment is obsolete: true

Hello Sreshta, how is it going on?
Do you need any help to push this over the finish line?

Flags: needinfo?(sresthasrivastava.ss)

clearing assignee

Type: enhancement → task
Flags: needinfo?(sresthasrivastava.ss)

I think Nicolas meant to clear the assignee field, but didn't actually do it. Let me fix this.

Assignee: sresthasrivastava.ss → nobody
Status: ASSIGNED → NEW

Is this still a bug? Can I retake it if it is still open?

Hello Armando, yes this is still pending, so I assigned it to you :)
You can follow instructions in Comment 2, they should still be accurate.
Thanks!

Assignee: nobody → armando.ferreira
Status: NEW → ASSIGNED

Thanks I will start working on this! ;)

Hi just to give a quick update that I'm still working on this, just I'm a little too slow since I'm doing this in my free times after work. QQ How can I check that all the paths are successfully updated? Just by building Firefox?

(In reply to Armando Ferreira from comment #22)

Hi just to give a quick update that I'm still working on this, just I'm a little too slow since I'm doing this in my free times after work. QQ How can I check that all the paths are successfully updated? Just by building Firefox?

No worries at all, it's hard to work on a first patch on this huge code base!
So yes, if you manage to build and run firefox, this is a good sign.
Then you can open the webconsole and try to log a few things there.
The next step would be to run the node tests that we have by doing :

cd devtools/client/webconsole/test/
yarn
yarn test

They might break because we are requiring component from there, and the paths should be updated.

Let me know how it goes
And again, don't worry about taking your time, we're not in a hurry :)

One last thing, next time you want to ask a question, check the Request information from checkbox, so I'll get a more prominent notification :)

Thanks a lot!

Attachment #9040167 - Attachment is obsolete: true
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fa93fa33b893
Re-organise devtools/client/webconsole/components/ folder. r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
You need to log in before you can comment on or make changes to this bug.