Currently, home.js looks too large and became difficult to debug and solve issues so split home.js into different components.
Description
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Gopavasanth | T249993 Split home.js into different components | |||
| Resolved | Khr2003 | T290241 Refactor main code |
Event Timeline
@Gopavasanth Hi! As this task is tagged with good first task, it might be ideal if you can add to the task description, what home.js component does, and how to go about splitting it. For the scope of this task, contributing to one new component might be reasonable that you can advise what that should be. Please update the task description accordingly. Thanks!
Can I do this task, in the direction of contributing for the project idea https://phabricator.wikimedia.org/T164307 ?
We can split the code into many components for instance Header, Footer, Player, VideoSettings (menu) and many more, For the acceptance of the code, at least one component should be made without any code duplication.
Hello @Hsync7 If you are working on this task, Please claim the task and feel free to ask any questions if you have !
Change 630981 had a related patch set uploaded (by Sohom Datta; owner: Sohom Datta):
[labs/tools/VideoCutTool@master] Seperated header, footer, notificationsUtils and VideoSettings
Change 630981 merged by jenkins-bot:
[labs/tools/VideoCutTool@master] Seperated header, footer, notificationsUtils and VideoSettings
Thanks, @Soda for your initial commit, As this ticket, can have many instances removing the assignee so anyone else can also work on this, please feel free to create another patch if you are interested to work on this ticket again :)
Hi @Gopavasanth. I'm a first-time contributor at WikiMedia. The file seems humongous. Can you help me out by suggesting some lines that can be moved and I'll move them initially? My initial thought is to move some functions into another directory called "utils" and then import them. Does that sound like something that'll help? Thanks!
@SarthakKundra Welcome! Yes, that sounds like a great plan and you can go ahead with that :)
Hi @Gopavasanth. Getting the following error on running npm run start. Can you help me with this? I did run npm audit fix to fix the vulnerabilities caused by old packages being used.
Hi @SarthakKundra, Can you try removing the node_modules directory and re-install them again with npm install?
Already tried that twice. First i thought it was because I did it on a different node version. Then did it after switching to the correct node version using nvm
@Gopavasanth For some reason a fresh clone seems to work fine. Can you please guide me about the process of creating a PR? Should I create it on the Github mirror only? If not, it'll be great if you can guide me a bit. Thanks!
Hi @SarthakKundra, please see https://www.mediawiki.org/wiki/New_Developers and https://www.mediawiki.org/wiki/Gerrit/Tutorial - thanks a lot!
@SarthakKundra: Hi, please see https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines - thanks.
Done @Aklapper although I can't see it being commented here by a bot. Maybe it'll take some time.
@SarthakKundra: See https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines - there should not be an empty line between Bug and Change-Id. Also note "Use the imperative mood in your subject line."; currently it's past tense.
Change 660995 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions from home.js to a different file
Hi @SarthakKundra are you sure you tested your patch locally and it works? My console gives
Failed to compile. ./src/components/home.js Line 1166:59: 'decodeTime' is not defined no-undef Line 1211:59: 'decodeTime' is not defined no-undef Search for the keywords to learn more about each error.
Change 662079 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js
Change 662080 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js
Change 662112 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js
Change 662660 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js
Change 660995 abandoned by SarthakKundra:
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js
Reason:
Fixed flaws in a new PR
Hi @Gopavasanth . I would like to work on this task . Is this task still open ? Thanks !
Hey @Uditdesai2206 I'm actively working on this. It'll be great to take up another task to avoid conflicting patches. Thanks :)
Change 662660 merged by jenkins-bot:
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js
Change 662112 abandoned by Gopavasanth:
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js
Reason:
Suppressed by https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/ /662660/
Change 662080 abandoned by Gopavasanth:
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js
Reason:
Suppressed by https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/ /662660/
Change 662079 abandoned by Gopavasanth:
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js
Reason:
Suppressed by https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/ /662660/
Hi Naman, Thanks for your interest on this ticket, unfortunately this is not open at the moment due to T290241: Refactor main code
