Closed
Bug 1452064
Opened 7 years ago
Closed 7 years ago
CamelCase all React component files in /devtools/client/dom/content/components/
Categories
(DevTools :: DOM, enhancement, P3)
DevTools
DOM
Tracking
(firefox61 fixed)
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: jdescottes, Assigned: migueluseche)
References
Details
(Keywords: good-first-bug)
Attachments
(1 file, 1 obsolete file)
2.48 KB,
patch
|
Honza
:
review+
|
Details | Diff | Splinter Review |
We recently agreed to name our React component file names using the CamelCase convention.
However the files in /devtools/client/dom/content/components do not follow this convention, so let's rename them.
This is an easy fix, but make sure you read the contribution docs before starting: http://docs.firefox-dev.tools/ so that you can build Firefox locally and verify that things still work after your change.
After building, just running the built Firefox and opening the memory panel (by first enabling it from the settings panel) should be enough of a test that things still work.
Reporter | ||
Comment 1•7 years ago
|
||
Sorry for copy pasting a summary, I assumed the last part was just a set of generic instructions:
"After building, just running the built Firefox and opening the memory panel (by first enabling it from the settings panel) should be enough of a test that things still work."
Should be
"After building, just running the built Firefox and opening the DOM panel (by first enabling it from the settings panel) should be enough of a test that things still work"
Assignee | ||
Comment 2•7 years ago
|
||
May I take this?
Reporter | ||
Comment 3•7 years ago
|
||
Sure, thanks for jumping on this :) Assigned to you!
Assignee: nobody → migueluseche
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•7 years ago
|
||
Here's the patch. I tested by doing memory snaps and it worked.
Comment 5•7 years ago
|
||
@migueluseche: The patch looks good, but the commit message is somehow wrong and there is no reviewer associated (not recognized by Bugzilla). It might be perhaps the slashes in the message...
You might try:
Bug 1452064 - CamelCase all React component files for DOM panel. r=pbro
Also, :pbro is on PTO, so you can ask me for review if you want to get it faster.
In such case:
Bug 1452064 - CamelCase all React component files for DOM panel. r=Honza
Honza
Assignee | ||
Comment 6•7 years ago
|
||
Here's the updated patch with suggested commit message.
Assignee | ||
Comment 7•7 years ago
|
||
Sorry, I forgot to mark previous patch as obsolete now I can't find how to do it.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(odvarko)
Reporter | ||
Comment 8•7 years ago
|
||
(In reply to migueluseche from comment #7)
> Sorry, I forgot to mark previous patch as obsolete now I can't find how to
> do it.
Steps:
- go to the details page for the attachment to mark as obsolete
here: https://bugzilla.mozilla.org/attachment.cgi?id=8967989&action=edit
- click on "Edit details" (next to "[patch] bug-1452064.patch")
- check "obsolete" checkbox
- submit
Comment 9•7 years ago
|
||
Comment on attachment 8968522 [details] [diff] [review]
Updated patch with new commit message
Review of attachment 8968522 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me, thanks for helping with this!
R+
Honza
Attachment #8968522 -
Flags: review+
Comment 10•7 years ago
|
||
@migueluseche: two things you should do now
1) Mark the previous patch as obsolete (see instructions in comment #8)
2) Add `checkin-needed` keyword into the Keywords field. It will ensure that somebody will land your patch into mozilla-central (m-c) repo. In order to do this, click the blue 'Edit Bug' button at the top of this page and search for Keywords field in section Tracking.
Thanks,
Honza
Flags: needinfo?(odvarko) → needinfo?(migueluseche)
Assignee | ||
Updated•7 years ago
|
Attachment #8967989 -
Attachment is obsolete: true
Flags: needinfo?(migueluseche)
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 11•7 years ago
|
||
Done, thanks for your patience.
Comment 12•7 years ago
|
||
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19faa628ee03
CamelCase all React component files for DOM panel. r=Honza
Keywords: checkin-needed
Comment 13•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•