940016 - Add Reload And Run to the context menu

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect, P3)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

We apparently forgot to add Reload And Run to the context menu, even though it is present in the regular Execute menu.

Comment 1

[Dan Koepke]

Attachment: bug940016.diff

This is my first submission. I think I have a properly formatted patch, and I know I need to ask for the patch to be reviewed, but I'm not sure how. Via flags? With this comment?

Comment 2

[Anton Kovalyov (:anton)]

Thanks Dan!

When you add an attachment there is an option to ask for a review: http://cl.ly/image/320Y3N2P3508. Alternatively, you can also click on the [details] link next to your attachment and ask for a review from there.

The patch looks good to me but I'll let Brandon to do the final call since he's a mentor here.

Comment 3

[Dan Koepke]

(In reply to Anton Kovalyov (:anton, @valueof) from comment #2)
> Thanks Dan!
> 
> When you add an attachment there is an option to ask for a review:
> http://cl.ly/image/320Y3N2P3508. Alternatively, you can also click on the
> [details] link next to your attachment and ask for a review from there.

Sounds good. Thanks Anton!

Comment 4

[Brandon Benvie [:benvie]]

Comment on attachment 8335117 [details] [diff] [review]
bug940016.diff

Review of attachment 8335117 [details] [diff] [review]:
-----------------------------------------------------------------

The change looks good, however we should also make sure this is tested. There's a test [1] that already runs through the menus, so you just have to add the menu to the list to test. r+ with it added to the test.

[1] http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/test/browser_scratchpad_ui.js#31

Comment 5

[Dan Koepke]

Attachment: bug940016+test.diff

./mach mochitest-browser browser/devtools/scratchpad/test

Browser Chrome Test Summary
 	Passed: 311
 	Failed: 0
 	Todo: 0
 
 *** End BrowserChrome Test Results ***

Comment 6

[Brandon Benvie [:benvie]]

Hmm... the patch has some weird extra formatting in it. Take a look at the diff. Can you fix that?

Comment 7

[Dan Koepke]

Should each file have it's own patch? I'm using git and the git-patch-to-hg-patch tool as mentioned on https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F but it only converts the first patch.

Comment 8

[Dan Koepke]

Attachment: bug940016+test-v2.diff

Comment 9

[Dan Koepke]

(In reply to Dan Koepke from comment #7)
> Should each file have it's own patch?...

I think I answered my own question. The most recent patch I attached has both files and the diff tool shows everything like it seems like it should.

Comment 10

[Brandon Benvie [:benvie]]

Comment on attachment 8335764 [details] [diff] [review]
bug940016+test-v2.diff

Review of attachment 8335764 [details] [diff] [review]:
-----------------------------------------------------------------

That's better! Looks good to me!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/1638fd317c46

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1638fd317c46