1554885 - Simplify devtools/client/netmonitor/src/widgets/RequestListContextMenu.js and remove eslint-disable complexity

Status: RESOLVED FIXED

(DevTools :: Netmonitor, task, P3)

Description

[Patrick Brosset <:pbro>]

This file contains a special eslint comment to temporarily disable the complexity eslint rule since it contains one or more functions that exceed the maximum complexity threshold. The code should be simplified, and the comment removed. Link to the code in question: https://searchfox.org/mozilla-central/search?q=eslint-disable+complexity&case=false&regexp=false&path=devtools%2Fclient%2Fnetmonitor%2Fsrc%2Fwidgets%2FRequestListContextMenu.js

Comment 1

[Atique Ahmed Ziad [:atiqueahmedziad]]

Can I work on it ?

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Assigned to you, thanks for the help!

Honza

Comment 3

[Atique Ahmed Ziad [:atiqueahmedziad]]

Thanks Honza.

I found empty results from the given link in comment 0 , however I got something here - https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/widgets/RequestListContextMenu.js#48 - is this the right place to work on?

(In reply to Patrick Brosset <:pbro> from comment #0)

This file contains a special eslint comment to temporarily disable the complexity eslint rule since it contains one or more functions that exceed the maximum complexity threshold. The code should be simplified, and the comment removed. Link to the code in question: https://searchfox.org/mozilla-central/search?q=eslint-disable+complexity&case=false&regexp=false&path=devtools%2Fclient%2Fnetmonitor%2Fsrc%2Fwidgets%2FRequestListContextMenu.js

Also I need an explanation with the following line from comment 0 - "The code should be simplified". What should I focus on to make the code simplified ?

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Sorry for the delay, this slipped out of my TODO list.

however I got something here - https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/widgets/RequestListContextMenu.js#48 - is this the right place to work on?

Correct

You can remove the line // eslint-disable-next-line complexity and run eslint from the command line:

$ mach eslint devtools/client/netmonitor/src/widgets/
c:/src/mozilla.org/mozilla-central/devtools/client/netmonitor/src/widgets/RequestListContextMenu.js
  48:7  error  Method 'open' has a complexity of 26. Maximum allowed is 20.  complexity (eslint)

✖ 1 problem (1 error, 0 warnings)

The complexity of the code is big you need to simplify it.

Atique, are you still interested in fixing this?

Honza

Comment 5

[Atique Ahmed Ziad [:atiqueahmedziad]]

Hello Honza,

Thanks for the reply. Yes, I am still interested to fix this issue.

$ mach eslint devtools/client/netmonitor/src/widgets/
c:/src/mozilla.org/mozilla-central/devtools/client/netmonitor/src/widgets/RequestListContextMenu.js
48:7 error Method 'open' has a complexity of 26. Maximum allowed is 20. complexity (eslint)

✖ 1 problem (1 error, 0 warnings)

The complexity of the code is big you need to simplify it.

I could reproduce the error locally removing the that comment. I am thinking about how to fix the complexity here. Would you mind giving a hint for it ?

Thanks
Atique

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Good question, what about moving the submenu items creation (copySubmenu array) into another function?
That could reduce the complexity a bit.

Perhaps we could have:

• open (calls createMenu)
• createMenu (calls createCopySubMenu)
• createCopySubMenu

Honza

Comment 7

[Atique Ahmed Ziad [:atiqueahmedziad]]

Attachment: Bug 1554885 - Simplify devtools RequestListContextMenu and remove eslint comment r=Honza

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51b0a3f42b19
Simplify devtools RequestListContextMenu and remove eslint comment r=Honza

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/51b0a3f42b19