You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2016/02/17 01:31:29 UTC

Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/
-----------------------------------------------------------

Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.


Bugs: MESOS-3929
    https://issues.apache.org/jira/browse/MESOS-3929


Repository: mesos


Description
-------

This script allows committers to push locally applied review chain to the ASF
git repo and mark the reviews as submitted.


Diffs
-----

  support/push-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/43552/diff/


Testing
-------

Tested locally.


Thanks,

Vinod Kone


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Kevin Klues <kl...@gmail.com>.

> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 108-112
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line108>
> >
> >     As we close the review, we should also post a comment to reviewboard with the commit message we actually pushed to master.
> >     
> >     You can use the --description flag:
> >     
> >     https://www.reviewboard.org/docs/rbtools/dev/rbt/commands/close/
> 
> Vinod Kone wrote:
>     I'll make a TODO for now as committers don't currently/always set the commit message on RB.
> 
> Kevin Klues wrote:
>     I think we should add the option as a command line flag now, so that people who like to include the commit message when closing can still take advantage of this script.  If we later decide that *everyone* should always include the commit message, we can remove the flag and force it to be set.
> 
> Vinod Kone wrote:
>     I am suggesting a TODO because it needs some non-trivial code change to get it to work. `get_reviews()` currently read log line by line.
>     
>     Also, note that people can still put a commit message in the description by directly going to RB after running this script. So I see this script as a strict improvement to the current workflow.

OK.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review119507
-----------------------------------------------------------


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Vinod Kone <vi...@gmail.com>.

> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > In general, I prefer scripts with a bunch of helper functions and a compact main() that steps through each of them. I'm not sure what the general concensus for the Mesos code base is, but I generally find this easier to walk through.  Example: support/generate-endpoint-help.py

In our C++ code base we generally prefer procedural style as it requires fewer context switches. We haven't officially defined the style for Python code in our code base, but I would guess we want to be similar. 

That said if something makes sense to be a function (e.g, reuse) we should extract it into a function. I've added a main and refactored a bit. Let me know if it looks ok.


> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 27-30
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line27>
> >
> >     Do we have guidelines for indentation here?  When writing the support/generate-endpoints-help.py script, bmahler suggested something different than what you have here.

I'm trying to be close to our C++ style. This is how it is in apply-reviews.py too. 

What did bmahler suggest?


> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 32-39
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line32>
> >
> >     If the tracking branch is reauired to be of the form `remote/branch` (as mentioned in a later comment), what do we need the `remote` flag for?

"tracking branch" is mainly required to figure out the new commits to push. Technically it can be just "master". "remote" is to figure out the upstream to push to. If remote is not specified, we try to deduce it from "tracking branch", in which case we expect it to be a "remote tracking branch". 

But I agree it's all confusing. I will kill both "--remote" and "--tracking_branch" and expect users to run this from "master" branch for simplicity.


> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, line 52
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line52>
> >
> >     We need some sort of way to enforce that the tracking branch is always of the form `remote/branch`. Otherwise, if we run e.g.
> >     
> >     ```
> >     $ klueska@c99:~/projects/mesos$ support/push-reviews.py -t master
> >     ```
> >     
> >     remote gets set to `master` here, which obviously breaks things later on.

see above.


> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 108-112
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line108>
> >
> >     As we close the review, we should also post a comment to reviewboard with the commit message we actually pushed to master.
> >     
> >     You can use the --description flag:
> >     
> >     https://www.reviewboard.org/docs/rbtools/dev/rbt/commands/close/

I'll make a TODO for now as committers don't currently/always set the commit message on RB.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review119507
-----------------------------------------------------------


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Vinod Kone <vi...@gmail.com>.

> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 108-112
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line108>
> >
> >     As we close the review, we should also post a comment to reviewboard with the commit message we actually pushed to master.
> >     
> >     You can use the --description flag:
> >     
> >     https://www.reviewboard.org/docs/rbtools/dev/rbt/commands/close/
> 
> Vinod Kone wrote:
>     I'll make a TODO for now as committers don't currently/always set the commit message on RB.
> 
> Kevin Klues wrote:
>     I think we should add the option as a command line flag now, so that people who like to include the commit message when closing can still take advantage of this script.  If we later decide that *everyone* should always include the commit message, we can remove the flag and force it to be set.

I am suggesting a TODO because it needs some non-trivial code change to get it to work. `get_reviews()` currently read log line by line.

Also, note that people can still put a commit message in the description by directly going to RB after running this script. So I see this script as a strict improvement to the current workflow.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review119507
-----------------------------------------------------------


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Kevin Klues <kl...@gmail.com>.

> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > In general, I prefer scripts with a bunch of helper functions and a compact main() that steps through each of them. I'm not sure what the general concensus for the Mesos code base is, but I generally find this easier to walk through.  Example: support/generate-endpoint-help.py
> 
> Vinod Kone wrote:
>     In our C++ code base we generally prefer procedural style as it requires fewer context switches. We haven't officially defined the style for Python code in our code base, but I would guess we want to be similar. 
>     
>     That said if something makes sense to be a function (e.g, reuse) we should extract it into a function. I've added a main and refactored a bit. Let me know if it looks ok.

Looks great.


> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 27-30
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line27>
> >
> >     Do we have guidelines for indentation here?  When writing the support/generate-endpoints-help.py script, bmahler suggested something different than what you have here.
> 
> Vinod Kone wrote:
>     I'm trying to be close to our C++ style. This is how it is in apply-reviews.py too. 
>     
>     What did bmahler suggest?

He had suggested putting all parameters on the next line, indedented by 4 spaces instead of directly after the pranthesis. e.g.

```
parser.add_argument(
    '-n',
    '--dry-run',
    action='store_true',
    help='Perform a dry run.')
```


> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 32-39
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line32>
> >
> >     If the tracking branch is reauired to be of the form `remote/branch` (as mentioned in a later comment), what do we need the `remote` flag for?
> 
> Vinod Kone wrote:
>     "tracking branch" is mainly required to figure out the new commits to push. Technically it can be just "master". "remote" is to figure out the upstream to push to. If remote is not specified, we try to deduce it from "tracking branch", in which case we expect it to be a "remote tracking branch". 
>     
>     But I agree it's all confusing. I will kill both "--remote" and "--tracking_branch" and expect users to run this from "master" branch for simplicity.

Yeah, this is simpler because otherwise you could run into a situation where tracking branch was something like "klueska/my-branch" and remote was set to "origin", causing a conflict that we would need to check for.  It's hard for me to imagine a situation where you would be wanting to run this script anywhere other than master, so this change makes sense.


> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 108-112
> > <https://reviews.apache.org/r/43552/diff/1/?file=1252228#file1252228line108>
> >
> >     As we close the review, we should also post a comment to reviewboard with the commit message we actually pushed to master.
> >     
> >     You can use the --description flag:
> >     
> >     https://www.reviewboard.org/docs/rbtools/dev/rbt/commands/close/
> 
> Vinod Kone wrote:
>     I'll make a TODO for now as committers don't currently/always set the commit message on RB.

I think we should add the option as a command line flag now, so that people who like to include the commit message when closing can still take advantage of this script.  If we later decide that *everyone* should always include the commit message, we can remove the flag and force it to be set.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review119507
-----------------------------------------------------------


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review119507
-----------------------------------------------------------



In general, I prefer scripts with a bunch of helper functions and a compact main() that steps through each of them. I'm not sure what the general concensus for the Mesos code base is, but I generally find this easier to walk through.  Example: support/generate-endpoint-help.py


support/push-reviews.py (line 19)
<https://reviews.apache.org/r/43552/#comment180861>

    need to import sys



support/push-reviews.py (lines 27 - 30)
<https://reviews.apache.org/r/43552/#comment180868>

    Do we have guidelines for indentation here?  When writing the support/generate-endpoints-help.py script, bmahler suggested something different than what you have here.



support/push-reviews.py (lines 32 - 39)
<https://reviews.apache.org/r/43552/#comment180865>

    If the tracking branch is reauired to be of the form `remote/branch` (as mentioned in a later comment), what do we need the `remote` flag for?



support/push-reviews.py (line 52)
<https://reviews.apache.org/r/43552/#comment180864>

    We need some sort of way to enforce that the tracking branch is always of the form `remote/branch`. Otherwise, if we run e.g.
    
    ```
    $ klueska@c99:~/projects/mesos$ support/push-reviews.py -t master
    ```
    
    remote gets set to `master` here, which obviously breaks things later on.



support/push-reviews.py (lines 108 - 112)
<https://reviews.apache.org/r/43552/#comment180867>

    As we close the review, we should also post a comment to reviewboard with the commit message we actually pushed to master.
    
    You can use the --description flag:
    
    https://www.reviewboard.org/docs/rbtools/dev/rbt/commands/close/


- Kevin Klues


On Feb. 17, 2016, 12:31 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2016, 12:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review119420
-----------------------------------------------------------



Patch looks great!

Reviews applied: [43552]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 17, 2016, 12:31 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2016, 12:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review120133
-----------------------------------------------------------



Patch looks great!

Reviews applied: [43552]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Vinod Kone <vi...@gmail.com>.

> On Feb. 24, 2016, 5:16 a.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 123-125
> > <https://reviews.apache.org/r/43552/diff/2/?file=1263874#file1263874line123>
> >
> >     Ah, I see it now. There is another call to `if not options['dry_run']` inside the `close_reviews()` call which protects it from actually closing it on rb. Seems like this could be consolidated somehow, since it's a little confusing at first glance, but may not be worth the effort.

I've removed the "if reviews:" to avoid confusion.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review120452
-----------------------------------------------------------


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review120452
-----------------------------------------------------------




support/push-reviews.py (lines 123 - 125)
<https://reviews.apache.org/r/43552/#comment181907>

    Ah, I see it now. There is another call to `if not options['dry_run']` inside the `close_reviews()` call which protects it from actually closing it on rb. Seems like this could be consolidated somehow, since it's a little confusing at first glance, but may not be worth the effort.


- Kevin Klues


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review120977
-----------------------------------------------------------


Ship it!




Ship It!

- Kevin Klues


On Feb. 24, 2016, 7:23 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 7:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review120633
-----------------------------------------------------------



Patch looks great!

Reviews applied: [43552]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 24, 2016, 7:23 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 7:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/
-----------------------------------------------------------

(Updated Feb. 24, 2016, 7:23 p.m.)


Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.


Changes
-------

kevin's.


Bugs: MESOS-3929
    https://issues.apache.org/jira/browse/MESOS-3929


Repository: mesos


Description
-------

This script allows committers to push locally applied review chain to the ASF
git repo and mark the reviews as submitted.


Diffs (updated)
-----

  support/push-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/43552/diff/


Testing
-------

Tested locally.


Thanks,

Vinod Kone


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Vinod Kone <vi...@gmail.com>.

> On Feb. 23, 2016, 12:44 a.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 14-15
> > <https://reviews.apache.org/r/43552/diff/2/?file=1263874#file1263874line14>
> >
> >     I would at least pass this as a flag right now rather than jsut adding a TODO. Otherwise people that prefer to close with the commit message won't be able to use your script at all.

Not sure what you mean by pass this as a flag? Do you mean add a command line argument to the script (--say post_to_JIRA) that is a no-op? Or do you want me to write code that speaks JIRA API? Also I don't understand why people who prefer to close with a commit message can't use this script. See my comment in the previous reply to your comment.


> On Feb. 23, 2016, 12:44 a.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 123-125
> > <https://reviews.apache.org/r/43552/diff/2/?file=1263874#file1263874line123>
> >
> >     Shouldn't this also be protected by the `if not options['dry_run']` check?

The only reason was to print the 'Closing review...' irrespective of the dry run option.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review120223
-----------------------------------------------------------


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/#review120223
-----------------------------------------------------------




support/push-reviews.py (lines 14 - 15)
<https://reviews.apache.org/r/43552/#comment181647>

    I would at least pass this as a flag right now rather than jsut adding a TODO. Otherwise people that prefer to close with the commit message won't be able to use your script at all.



support/push-reviews.py (lines 123 - 125)
<https://reviews.apache.org/r/43552/#comment181648>

    Shouldn't this also be protected by the `if not options['dry_run']` check?


- Kevin Klues


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
>     https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -----
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43552/
-----------------------------------------------------------

(Updated Feb. 22, 2016, 4:52 a.m.)


Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.


Changes
-------

kevin's comments.


Bugs: MESOS-3929
    https://issues.apache.org/jira/browse/MESOS-3929


Repository: mesos


Description
-------

This script allows committers to push locally applied review chain to the ASF
git repo and mark the reviews as submitted.


Diffs (updated)
-----

  support/push-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/43552/diff/


Testing
-------

Tested locally.


Thanks,

Vinod Kone