Closed
Bug 1303708
Opened 7 years ago
Closed 7 years ago
Remove unused JSID_IS_ZERO function.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox51 | --- | fix-optional |
firefox52 | --- | fixed |
People
(Reporter: arai, Assigned: userajay0, Mentored)
Details
(Keywords: good-first-bug, Whiteboard: [good first bug] [lang=c++])
Attachments
(2 files, 1 obsolete file)
5.73 KB,
patch
|
Details | Diff | Splinter Review | |
58 bytes,
text/x-review-board-request
|
arai
:
review+
|
Details |
JSID_IS_ZERO function was added 5 years ago by bug 601457 to catch unexpected case, and it's not used anymore. we could remove it now. (if not, we would need to add a comment that explains the purpose of the function) https://dxr.mozilla.org/mozilla-central/rev/f0f15b7c6aa77a0c5750918aa0a1cb3dc82185bc/js/public/Id.h#72 > static MOZ_ALWAYS_INLINE bool > JSID_IS_ZERO(jsid id) > { > return JSID_BITS(id) == 0; > }
Comment 1•7 years ago
|
||
Hello ! I am new here , Can i request to get this bug assigned to me ?
Reporter | ||
Comment 2•7 years ago
|
||
Thank you for your comment :) I'll assign bug when a patch is posted. if you have any question related to building, testing, etc, feel free to ask here or in IRC #introduction channel.
Comment 3•7 years ago
|
||
Although i do understand what needs to be done , can you tell me how do i go about providing a patch ? accessing this bug and rectifying it . thanks for your help .
Reporter | ||
Comment 4•7 years ago
|
||
you can provide a patch by either * push your changeset to review repository * export your patch as a patch file and attach it to this bug about whole workflow, these documents will help https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/firefoxworkflow.html
Comment 5•7 years ago
|
||
Hi! I'm new here and finding my way about. Can you help my way around here. Thank you :)
Reporter | ||
Comment 6•7 years ago
|
||
(In reply to Amal Santhosh from comment #5) > Hi! I'm new here and finding my way about. > Can you help my way around here. > Thank you :) pass2pawan already commented they're going to work on this. I filed bug 1304167 if you're looking for C++ good-first-bug.
Comment 7•7 years ago
|
||
Hello, I am a first timer. Please review the patch and tell me if something needs to be changed. Thanks!
Attachment #8793169 -
Flags: review?(arai.unmht)
Comment 8•7 years ago
|
||
(In reply to vinayakagarwal6996 from comment #7) > Created attachment 8793169 [details] [diff] [review] > Patch file > > Hello, > I am a first timer. Please review the patch and tell me if something needs > to be changed. > > Thanks! Please delete this. I'm sorry. Wrong bug. :(
Reporter | ||
Comment 9•7 years ago
|
||
Comment on attachment 8793169 [details] [diff] [review] Patch file per comment #8.
Attachment #8793169 -
Attachment is obsolete: true
Attachment #8793169 -
Flags: review?(arai.unmht)
Comment 10•7 years ago
|
||
Tooru , i have the build in mercurial , is there any comprehensive guide which i can follow to track down the code and debug and patch it ? or could you help me in anyway for doing that?
Reporter | ||
Comment 11•7 years ago
|
||
the file is in js/public/Id.h, under mozilla-central (or mozilla-unified) about debugging and patching, what's your question?
Comment 12•7 years ago
|
||
I found the file and saved it in my build . now after this how do i create a patch ? what commands do i run on mercurial.
Reporter | ||
Comment 13•7 years ago
|
||
here's guide for workflow https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/firefoxworkflow.html about creating patch, you need to create a changeset, by hg commit -m 'Bug 1303708 - Remove unused JSID_IS_ZERO function.' then, push it to review repository after configuring this: https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html#mozreview-user or, just export it and attach the file here: hg export -r . > PATH_TO_OUTPUT_PATCH_FILE.patch
Assignee | ||
Comment 14•7 years ago
|
||
Reporter | ||
Comment 15•7 years ago
|
||
(In reply to ajay.krishn97 from comment #14) > Created attachment 8793493 [details] [diff] [review] > The function to be removed has been removed you need to export your change as a patch file: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_-_Get_your_code_reviewed or submit it to review repository: https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/firefoxworkflow.html
Attachment #8793493 -
Flags: review?(arai.unmht)
Reporter | ||
Comment 16•7 years ago
|
||
Comment on attachment 8793493 [details] [diff] [review] The function to be removed has been removed please read comment #15 ;)
Attachment #8793493 -
Flags: review?(arai.unmht)
Assignee | ||
Comment 17•7 years ago
|
||
(In reply to Tooru Fujisawa [:arai] from comment #16) > Comment on attachment 8793493 [details] [diff] [review] > The function to be removed has been removed > > please read comment #15 ;) Is your Mozreview Username 'arai'? Thanks
Reporter | ||
Comment 18•7 years ago
|
||
(In reply to ajay.krishn97 from comment #17) > (In reply to Tooru Fujisawa [:arai] from comment #16) > > Comment on attachment 8793493 [details] [diff] [review] > > The function to be removed has been removed > > > > please read comment #15 ;) > > Is your Mozreview Username 'arai'? > > Thanks yes
Assignee | ||
Comment 19•7 years ago
|
||
After making the change I ran hg commit with following commit message r?arai Bug 1303708 Removed the function that had to be removed Then I got the ouput 'created new head' Then I ran hg push review and I get the message pushing to https://reviewboard-hg.mozilla.org/autoreview searching for appropriate review repository redirecting push to https://reviewboard-hg.mozilla.org/gecko abort: you must set mozilla.ircnick in your hgrc config file to your IRC nickname in order to perform code reviews Could you help me solve this issue Thanks
Reporter | ||
Comment 20•7 years ago
|
||
here's documentation https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/install-mercurial.html#mozreview-install-mercurial I think running `./mach mercurial-setup` will help
Comment hidden (mozreview-request) |
Reporter | ||
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8793560 [details] Bug 1303708 - Remove unused JSID_IS_ZERO(jsid id) function. https://reviewboard.mozilla.org/r/80276/#review78918 Thank you for your patch :) the code change looks good. Can you change the commit message to follow this rule? https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Commit_message_restrictions so, put bug number and summary in the same line, and also describe the change more specifically, like: Bug 1303708 - Remove unused JSID_IS_ZERO function. or something similar.
Attachment #8793560 -
Flags: review?(arai.unmht)
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → ajay.krishn97
Status: NEW → ASSIGNED
Reporter | ||
Comment 23•7 years ago
|
||
pass2pawan, ping me if you're looking for other C++ bug.
Assignee | ||
Comment 24•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8793560 [details] Bug 1303708 - Remove unused JSID_IS_ZERO(jsid id) function. https://reviewboard.mozilla.org/r/80276/#review78918 So do I have to change the commit message(through 'hg commit --amend') and 'hg push review' it again or can I change it elsewhere(for example on reviewboard.mozilla.org) I'm sorry if the solution to my doubts are too obvious, I'm a new user here. Thanks
Reporter | ||
Comment 25•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8793560 [details] Bug 1303708 - Remove unused JSID_IS_ZERO(jsid id) function. https://reviewboard.mozilla.org/r/80276/#review78918 `hg commit --amend` will work, I think.
Comment hidden (mozreview-request) |
Reporter | ||
Comment 27•7 years ago
|
||
mozreview-review |
Comment on attachment 8793560 [details] Bug 1303708 - Remove unused JSID_IS_ZERO(jsid id) function. https://reviewboard.mozilla.org/r/80276/#review78928 Perfect :)
Attachment #8793560 -
Flags: review?(arai.unmht) → review+
Assignee | ||
Comment 28•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8793560 [details] Bug 1303708 - Remove unused JSID_IS_ZERO(jsid id) function. https://reviewboard.mozilla.org/r/80276/#review78928 Could you tell me what I have to do next? Thanks
Reporter | ||
Comment 29•7 years ago
|
||
I pushed your patch to try server. https://treeherder.mozilla.org/#/jobs?repo=try&revision=14ccea76c706 after it successfully finishes, the patch can be landed. if you don't have a permission to do so (I'm not sure who can/cannot, on MozReview), ping me.
Comment 30•7 years ago
|
||
Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/autoland/rev/ef5324ac2924 Remove unused JSID_IS_ZERO(jsid id) function. r=arai
Assignee | ||
Comment 31•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8793560 [details] Bug 1303708 - Remove unused JSID_IS_ZERO(jsid id) function. https://reviewboard.mozilla.org/r/80276/#review78928 Is there anything still left to do with regard to this bug or is the whole process finished
Reporter | ||
Comment 32•7 years ago
|
||
almost done. just wait for the patch be merged to mozilla-central. if something goes wrong (like it causes some issue), the patch will be backed out and you'll need to fix it, if nothing goes wrong, it's finished :)
Comment 33•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ef5324ac2924
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee | ||
Comment 34•7 years ago
|
||
(In reply to Tooru Fujisawa [:arai] from comment #32) > almost done. > just wait for the patch be merged to mozilla-central. > > if something goes wrong (like it causes some issue), the patch will be > backed out and you'll need to fix it, > if nothing goes wrong, it's finished :) So everything regarding resolving this bug is finished right? Thank you for all the help
Reporter | ||
Comment 35•7 years ago
|
||
Yes, finished. Thank you again for your contribution :)
Comment 36•6 years ago
|
||
Mark 51 as fix-optional. If it's worth uplift to 51, feel free to nominate it.
You need to log in
before you can comment on or make changes to this bug.
Description
•