1409694 - PreviousExperimentAddon.blocklistState will always return undefined

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P3)

Description

[Mark Banner (:standard8)]

In starting to look at turning on no-unused-expressions, I noticed this in Experiments.jsm:

http://searchfox.org/mozilla-central/rev/dca019c94bf3a840ed7ff50261483410cfece24f/browser/experiments/Experiments.jsm#2264-2266

  get blocklistState() {
    Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
  },

That function will always return undefined. I can't see an obvious case of this breaking something at the moment, but it could do.

Comment 1

[Chris H-C :chutten]

All for want of the word "return"

There are currently no Telemetry Experiments being run, so this can't cause problems too easily. But it's an easy fix and a good first bug.

Comment 2

[Vincent Hilton]

Can I take this one? I just started looking for bugs and this seems as good as any to make my first contribution.

Comment 3

[Chris H-C :chutten]

Please do! I have assigned the bug to you. Do let me know if you have any questions.

Comment 4

[Chris H-C :chutten]

Hello Vincent, are you still looking at this bug? Do you need any help?

Comment 5

[Ryan Leake]

If Vincent is no longer looking at this bug, I'm happy to fix it.

Comment 6

[Chris H-C :chutten]

Please, by all means do.

Vincent, if you'd like assistance choosing a different first bug, let me know here or on #telemetry on irc.mozilla.org and I can help you out (https://wiki.mozilla.org/Irc)

Comment 7

[Ryan Leake]

Attachment: Bug 1409694 - Add return statement for PreviousExperimentAddon.blocklistState.

Review commit: https://reviewboard.mozilla.org/r/201622/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201622/

Comment 8

[Chris H-C :chutten]

Comment on attachment 8930471 [details]
Bug 1409694 - Add return statement for PreviousExperimentAddon.blocklistState.

https://reviewboard.mozilla.org/r/201622/#review206986

Looks good!

Comment 9

[Chris H-C :chutten]

Thank you for your contribution, Ryan! I've marked it for checkin so it should be in Nightly in the next couple of days.

Would you like help finding the next bug to tackle (maybe one of these? https://www.joshmatthews.net/bugsahoy/?internals=1) or would you like to search around yourself?

Comment 10

[Ryan Leake]

(In reply to Chris H-C :chutten from comment #9)
> Thank you for your contribution, Ryan! I've marked it for checkin so it
> should be in Nightly in the next couple of days.
> 
> Would you like help finding the next bug to tackle (maybe one of these?
> https://www.joshmatthews.net/bugsahoy/?internals=1) or would you like to
> search around yourself?

Perfect, thanks Chris! I'd love some help finding another bug to tackle, I'm wanting to move on from "good first bugs" but am finding that there's quite a leap in difficulty as a beginner.

Comment 11

[Chris H-C :chutten]

It's true, we do make a special effort to make the good first bug as easy as possible (because learning the tools and process are hard enough without a difficult bug making it impossible :)

good next bugs/good second bugs are harder to come by. It's hard to strike that balance. Let's see what I can find you.

bug 1278247 is about renaming files. Not too complex, but moreso than adding "return" to one line.
bug 1289094 is really chewy. It involves messing with some JS <-> C++ APIs 
bug 1290256 involves a little bit about networking.
bug 1301365 is also on the chewy side. This will require reading some Python, and adding capabilities in C++

I'm not sure what your language strengths are, so I went for a mix. Hopefully there's something in here that catches your fancy.

Comment 12

[Ryan Leake]

(In reply to Chris H-C :chutten from comment #11)
> It's true, we do make a special effort to make the good first bug as easy as
> possible (because learning the tools and process are hard enough without a
> difficult bug making it impossible :)
> 
> good next bugs/good second bugs are harder to come by. It's hard to strike
> that balance. Let's see what I can find you.
> 
> bug 1278247 is about renaming files. Not too complex, but moreso than adding
> "return" to one line.
> bug 1289094 is really chewy. It involves messing with some JS <-> C++ APIs 
> bug 1290256 involves a little bit about networking.
> bug 1301365 is also on the chewy side. This will require reading some
> Python, and adding capabilities in C++
> 
> I'm not sure what your language strengths are, so I went for a mix.
> Hopefully there's something in here that catches your fancy.

Much appreciated, I'm sure I can make some contributions in there.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a75234ffae56
Add return statement for PreviousExperimentAddon.blocklistState. r=chutten

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/a75234ffae56

Comment 15

[Julien Cristau [:jcristau] (back April 22)]

Do we want this uplifted to 58?

Comment 16

[Chris H-C :chutten]

It's not harming anything (Telemetry Experiments are presently unused and are being removed (bug 1415284)), so I don't think there's any need.