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

Add a Help menuitem (#180) #204

Closed insaynasasin closed 9 years ago

insaynasasin commented 9 years ago

added a new link on the content.html page and updated the corresponding action in the main.js file

whimboo commented 9 years ago

So this PR will fix issue #180. (Mentioning this issue id will create a x-reference between the issue and PR)

insaynasasin commented 9 years ago

i have changed the menu entry and also the corresponding case as you said. is it fine now?

whimboo commented 9 years ago

Sorry, but I was a bit stressed out the last two weeks in fixing breakage of our CI systems. So I had lesser time for reviews. So let me have a look at your updated PR now.

insaynasasin commented 9 years ago

I am sorry for not testing. Please tell me after I have made changes to the actual code how would i test it? You mentioned in you comment that you tested it using ant run. So is this ant run used for such testing purposes?

whimboo commented 9 years ago

Yes, so in general it is necessary to test the code you have written. At least by doing it manually. Better is to write a test for. That is a bit harder, but always preferred. The existing tests for memchaser you can fine here. Those are low-level unit tests so far. In your case I don't think we can easily handle that, and may be not necessary.

To run the tests you have to use the tool ant. I'm not sure on which system you are working on, but e.g. on Linux it will be pre-installed. It's configuration is done in the build.xml file as located at the root of this repository. It contains all the possible commands incl. test (running automated tests) and run (start Firefox for manual testing). So you might want to have to install [ant](http://ant.apache.org/manual/install.html).

xabolcs commented 9 years ago

@insaynasasin do you have time to continue this pull request with addressing @whimboo's comment?

xabolcs commented 9 years ago

@whimboo, I'd like to continue this PR (ok, not this, but an another, based on this) after the other logging related PRs got merged.

whimboo commented 9 years ago

The logging PRs have been merged. So I think we are good to go here?

xabolcs commented 9 years ago

Sure.

Should I open a new PR, addressing your comments?

xabolcs commented 9 years ago

Merged to PR #220, comments addressed.

Closing.