Closed Bug 1150312 Opened 5 years ago Closed 5 years ago
Check if MOZ
_SHARK is still used and if not remove it
I asked a couple of people that are in the know and no one knew whether MOZ_SHARK is still needed. If it isn't it then it can be removed.
Probably worth asking dev.platform first, but if no one jumps up and down we should just remove it and see who complains :)
Whiteboard: [good first bug][mentor=:rstrong]
Still seeing email going by about this: can I get a go/no-go call on this so I can surface it to the community?
I haven't heard anything but speculation that someone somewhere might possibly be using it. In the absence of evidence I say we remove it.
As I understand it there are also alternatives. Go for it
If you or someone else would prefer to mentor this feel free to take over
Sheppy asked in the dev-platform thread whether the tenfourfox people should be contacted for this, and there is evidence that the tenfourfox people have seen the thread (http://tenfourfox.blogspot.jp/2015/04/beware-undead-snow-leopard.html), so I guess they would have complained if they were actively using shark. That said, better safe than sorry, so let's get an explicit statement. Cameron?
Yes, I'm aware. The MOZ_SHARK code appears to be specific to actually controlling Shark, whereas I just use Shark to attach to a running application and take samples, so I don't think removing it should affect TenFourFox work. However, I'd prefer this was not backported to 38ESR just in case, since I'm actively working on that port.
Whiteboard: [good first bug][mentor=:rstrong] → [good first bug]
Hi, I am seeing many MOZ_SHARK on source code . I want to make a patch for this, so could you tell me which one should be removed? Thanks : http://mxr.mozilla.org/mozilla-central/search?string=MOZ_SHARK
MOZ_SHARK is no longer needed so all.
Hi Robert, I submitted a patch. Could you review it and tell me if is ok? Thanks
Comment on attachment 8638094 [details] [diff] [review] 1150312.patch Nice... looks good to me. Let's also have a build peer review this.
Thanks, I will assign this bug to myself to keep it following.
Comment on attachment 8638094 [details] [diff] [review] 1150312.patch Review of attachment 8638094 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for your patch. "Check if MOZ_SHARK is still used and if not remove it" does not describe what the change is about. Please change this to e.g. "Remove MOZ_SHARK". ::: js/src/configure.in @@ -3231,5 @@ > - MOZ_PROFILING=1 > - AC_DEFINE(MOZ_SHARK) > -fi > - > -dnl ======================================================== There's an AC_SUSBT(MOZ_SHARK) leftover at line 3702.
Attachment #8638094 - Flags: review?(mh+mozilla) → review+
Attachment #8638255 - Flags: review?(mh+mozilla)
Hi Mike, I added the change you mentioned in the comment and changed the description in the patch. I usually put as description bug title, but I have not problem to change with a more descriptive text. Let me know if this one is ok and good to land. Thanks
Attachment #8638255 - Flags: review?(mh+mozilla) → review+
So, should I add checkin-needed keyword now? or there's something else to do?
That works and thanks again!
Attachment #8638094 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.