search

mozilla / memchaser

Firefox extension to chase the memory usage and garbage collector activity
https://wiki.mozilla.org/QA/Automation_Services/Projects/Addons/MemChaser
29 stars 23 forks source link

Switch from 'cfx' to 'jpm' for packaging (#203) #239

Closed xabolcs closed 8 years ago

xabolcs commented 8 years ago

Fix for #203.

xabolcs commented 8 years ago

Nightly and Aurora are green again! :+1:

xabolcs commented 8 years ago

Nightly and Aurora are green again! :+1:

Interestingly they fail locally.

xabolcs commented 8 years ago

A note about the new dependency of jpm is needed somewhere in README.

xabolcs commented 8 years ago

Interestingly they fail locally.

Ahh, too old jpm was used. With 1.0.3 all test pass locally.

whimboo commented 8 years ago

I believe its you who works on this PR and not me. :)

xabolcs commented 8 years ago

My two cent about this is that, I'm working on the issue - I provide the fix, and open the PR; you work on the PR - you review the PR.

whimboo commented 8 years ago

I wonder about the failure for esr38 and if that is related to the missing loop button error. I do not see that one of our tests is failing but it gets reported that way.

whimboo commented 8 years ago

My two cent about this is that, I'm working on the issue - I provide the fix, and open the PR; you work on the PR - you review the PR

The reviewer is not the assignee of a bug/issue/PR. It's always the person which contributes the code. And that's what you did and additionally spent a fair amount of time on it. So I don't want to steal that.

xabolcs commented 8 years ago

... loop button ...

What is it, where is it?

Anyway, the test killer exception is throw because loop.enabled is false in ESR38. Interestingly it was true for release 38.

xabolcs commented 8 years ago

So I don't want to steal that.

I used to steal the issue for that. :)

whimboo commented 8 years ago

... loop button ...

Anyway, the test killer exception is throw because loop.enabled is false in ESR38. Interestingly it was true for release 38.

@nils-ohlmeier do you know anything about that and why it was turned to false? Would be nice to know if that was wanted or by accident.

whimboo commented 8 years ago

I used to steal the issue for that. :)

On an issue several people can contribute to, but not to a PR. So you usually want to assign both to you. Also not sure how github calculates the contribution. So maybe you will miss lots of your actually work.

xabolcs commented 8 years ago

Also not sure how github calculates the contribution

Github says: Contributions to master, excluding merge commits, I'm in the TOP 3. :+1:

xabolcs commented 8 years ago

Landed as 120356a565cc5edb38dddff53135e1cce2b0abec.

xabolcs commented 8 years ago

A note about the new dependency of jpm is needed somewhere in README.

@whimboo this was missed.

xabolcs commented 8 years ago

@whimboo, do you have some good paragraph for README?

whimboo commented 8 years ago

Lets discuss this on the issue instead.