1360586 - Remove unused variable `maximized`

Status: RESOLVED FIXED

(Testing :: Marionette Client and Harness, enhancement)

Description

[David Burns :automatedtester]

There is an unused variable `maximized` at https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette_harness/tests/unit/test_window_maximize.py#81

Details for getting setup as a new Marionette contributor can be found at https://wiki.mozilla.org/User:Mjzffr/New_Contributors

Comment 1

[Jake H]

I would like to claim this bug if it is still available.
Thanks.

Comment 2

[David Burns :automatedtester]

Jake, go for it! If there isnt a patch in a next couple weeks I will unassign.

Comment 3

[Stephen Oliver]

Attachment: of Jake H's team in class)Bug 1360586: Removed the unused varialble on line 81 of test_window_maximize.py

I am a member of Jake H's team in class.
This is my first time submitting a patch, thank you for you patience!
Cheers,
Stephen

Comment 4

[David Burns :automatedtester]

Comment on attachment 8864274 [details] [diff] [review]
of Jake H's team in class)Bug 1360586: Removed the unused varialble on line 81 of test_window_maximize.py

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

Stephen,

Thanks for the patch but this is not touching the right file. I assume you uploaded the wrong patch :)

Comment 5

[Stephen Oliver]

I attempted to follow the MDN "How to submit a patch" Guide. I am not entirely sure what I did wrong at this point. I can see the the changes in hg status and hg diff commands.
Are there any further tips you can give me so I can fix my attachment?

Thanks,
Stephen

Comment 6

[David Burns :automatedtester]

(In reply to Stephen Oliver from comment #5)
> I attempted to follow the MDN "How to submit a patch" Guide. I am not
> entirely sure what I did wrong at this point. I can see the the changes in
> hg status and hg diff commands.
> Are there any further tips you can give me so I can fix my attachment?
> 
> Thanks,
> Stephen

There are details in http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html which mean you can just push to a repo. Its kinda like how PRs work in Github.

Comment 7

[bitout.cecelia]

Hi , I would like to work on this bug . But this is my first project and I can not find the files to remove this bug. So if you can please provide some more information on this?

Comment 8

[Jake H]

I am still working on it. We have the bug fix, but we (a few classmates) are working on the patch process. I think we understand it and should be submitting a patch during or after class tomorrow.

Comment 9

[Stephen Oliver]

Attachment: Bug 1360586: Removed unused maximized variable

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

Comment 10

[Stephen Oliver]

David, 

The patch should be submitted correctly now. Again, thank you for your patience with me on my first patch. Lastly, I am a member of Jake's team, just as a reminder.

Cheers!
-Stephen

Comment 11

[David Burns :automatedtester]

Comment on attachment 8865571 [details]
Bug 1360586: Removed unused maximized variable

https://reviewboard.mozilla.org/r/137200/#review140264

::: commit-message-d7e40:1
(Diff revision 1)
> +bug1360586 - 'Removed unused mazimized variable as per request' r=automatedtester

Please can you fix the typo and remove `as per request` then this is ready to go!

Comment 12

[Stephen Oliver]

David,

Sorry about that, I fixed it up as you asked. How do the patch content look? Does everything seem to be in-order?

Cheers!
-Stephen

Comment 13

[David Burns :automatedtester]

Stephen the code change looks good. I can't see your latest change, did you do |hg push review| with the updated commit message?

Comment 14

[Stephen Oliver]

No, I just changed it via an edit in the bugzilla interface. When I tried to hg push review it complained that there was already an entry and aborted. Tips?

Comment 15

[David Burns :automatedtester]

(In reply to Stephen Oliver from comment #14)
> No, I just changed it via an edit in the bugzilla interface. When I tried to
> hg push review it complained that there was already an entry and aborted.
> Tips?

So assuming your patch is the last commit do the following

hg histedit tip

where it says `pick` change that to `mess` for message and then exit the editor. Now change the message when it reopens. Save and exit.

Then you should be able to do |hg push review| again.

Comment 16

[Stephen Oliver]

Comment on attachment 8865571 [details]
Bug 1360586: Removed unused maximized variable

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/137200/diff/1-2/

Comment 17

[Stephen Oliver]

David,

Again I find myself having to apologize. I royally screwed up my patch while trying to fix it so I had to remake it. My hope is this time everything is in order.

Thanks again for your seemingly endless patience with my n00b self.
cheers!
-Stephen

Comment 18

[Stephen Oliver]

One last addition/side note:
Turns out my team ended up using mq for generating out patches somehow. Not sure if that effects your previous advise at all.

Comment 19

[David Burns :automatedtester]

Comment on attachment 8865571 [details]
Bug 1360586: Removed unused maximized variable

https://reviewboard.mozilla.org/r/137200/#review140726

Comment 20

[David Burns :automatedtester]

(In reply to Stephen Oliver from comment #18)
> One last addition/side note:
> Turns out my team ended up using mq for generating out patches somehow. Not
> sure if that effects your previous advise at all.

It does. MQ is deprecated in mercurial, I suggest learning bookmarks which are analogous to git branches... kinda

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ee68d8541c8
Removed unused maximized variable r=automatedtester

Comment 22

[Stephen Oliver]

(In reply to David Burns :automatedtester from comment #20)
> (In reply to Stephen Oliver from comment #18)
> > One last addition/side note:
> > Turns out my team ended up using mq for generating out patches somehow. Not
> > sure if that effects your previous advise at all.
> 
> It does. MQ is deprecated in mercurial, I suggest learning bookmarks which
> are analogous to git branches... kinda

Will do! I will make it a point to look for some documentation tomorrow in class for whatever we end up working on next.

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1ee68d8541c8

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Moving bugs for Marionette client due to component changes.