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

Status: RESOLVED FIXED

(DevTools :: Console, task, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

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

Comment 1

[SresthaSrivastava [:srestha]]

Can I take this up?

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

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

Comment 3

[SresthaSrivastava [:srestha]]

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

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(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)

Comment 5

[SresthaSrivastava [:srestha]]

Should I put ReverseSearchInput.css also in Input folder?

Comment 6

[SresthaSrivastava [:srestha]]

Attachment: Bug 1522104 - Re-organised devtools/client/webconsole/components r=nchevobbe

Comment 7

[SresthaSrivastava [:srestha]]

Please review

Comment 8

[Nicolas Chevobbe [: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 :)

Comment 9

[SresthaSrivastava [:srestha]]

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

Comment 10

[SresthaSrivastava [:srestha]]

Attachment: gh

Comment 11

[SresthaSrivastava [:srestha]]

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

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

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!

Comment 13

[SresthaSrivastava [:srestha]]

https://phabricator.services.mozilla.com/D18108
Hey, please look into this one

Comment 14

[SresthaSrivastava [:srestha]]

How do I abandon changes in phabricator?

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

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

Comment 17

[Nicolas Chevobbe [:nchevobbe]]

clearing assignee

Comment 18

[Patrick Brosset <:pbro>]

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

Comment 19

[Armando Ferreira]

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

Comment 20

[Nicolas Chevobbe [:nchevobbe]]

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!

Comment 21

[Armando Ferreira]

Thanks I will start working on this! ;)

Comment 22

[Armando Ferreira]

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?

Comment 23

[Nicolas Chevobbe [:nchevobbe]]

(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!

Comment 24

[Armando Ferreira]

Attachment: Bug 1522104 - Re-organise devtools/client/webconsole/components/ folder. r=nchevobbe

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa93fa33b893
Re-organise devtools/client/webconsole/components/ folder. r=nchevobbe

Comment 26

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/fa93fa33b893