[Twisted-Python] admin/pr_as_branch

[Jean-Paul Calderone]

Hello,

I didn't find any hints about the workflow surrounding the
admin/pr_as_branch tool so I invented one and wrote it up on the wiki:

https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76

Jean-Paul

[Glyph Lefkowitz]

Post by Jean-Paul Calderone
Hello,
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76 <https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76>
Jean-Paul

Thanks for writing this up. The workflow is evolving organically.

However, the "close the PR" step confuses me. If you do this, the original contributor won't be able to respond to feedback. What is the desired effect of this second PR?

-glyph

[Jean-Paul Calderone]

On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone <
Hello,
I didn't find any hints about the workflow surrounding the
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76
Jean-Paul
Thanks for writing this up. The workflow is evolving organically.
However, the "close the PR" step confuses me. If you do this, the
original contributor won't be able to respond to feedback. What is the
desired effect of this second PR?

The idea I heard is that it provides a place to hang build failure-related
review comments. I also had some vague notion that it would be the place
you'd look to see the complete CI results.

So. Where should further reviews go and where do you find CI results, if
you don't create a new PR?

Jean-Paul

-glyph
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Glyph Lefkowitz]

Post by Glyph Lefkowitz

Post by Jean-Paul Calderone
Hello,
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76 <https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76>
Jean-Paul

Thanks for writing this up. The workflow is evolving organically.
However, the "close the PR" step confuses me. If you do this, the original contributor won't be able to respond to feedback. What is the desired effect of this second PR?
The idea I heard is that it provides a place to hang build failure-related review comments. I also had some vague notion that it would be the place you'd look to see the complete CI results.
So. Where should further reviews go and where do you find CI results, if you don't create a new PR?

You find the CI results on the existing PR; they'll be displayed there. You just need to make sure that a branch in the repo has the exact same commit ID when you push it as the PR has. (Statuses are reported on commits, and triggered on pushes, so everything should just line up.)

As far as further reviews - on the existing PR too, same as usual.

You will need to run the script again when the contributor pushes their response to a review. One feature that would be useful is to not need to specify the branch name twice, but if you name the second pr_as_branch invocation differently, it just means one more branch that should be deleted when looking at git branch --merged.

-glyph

[Jean-Paul Calderone]

On Jan 22, 2017, at 5:24 PM, Jean-Paul Calderone <

On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone <
Hello,
I didn't find any hints about the workflow surrounding the
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76
Jean-Paul
Thanks for writing this up. The workflow is evolving organically.
However, the "close the PR" step confuses me. If you do this, the
original contributor won't be able to respond to feedback. What is the
desired effect of this second PR?

The idea I heard is that it provides a place to hang build failure-related
review comments. I also had some vague notion that it would be the place
you'd look to see the complete CI results.
So. Where should further reviews go and where do you find CI results, if
you don't create a new PR?
You find the CI results on the existing PR; they'll be displayed there.
You just need to make sure that a branch in the repo has the exact same
commit ID when you push it as the PR has. (Statuses are reported on
commits, and triggered on pushes, so everything should just line up.)

After the branch in the repo is all set (which I assume pr_as_branch will
do), does CI notice without further help and start the necessary builds?

As far as further reviews - on the existing PR too, same as usual.
You will need to run the script again when the contributor pushes their
response to a review. One feature that would be useful is to not need to
specify the branch name twice, but if you name the second pr_as_branch
invocation differently, it just means one more branch that should be
deleted when looking at git branch --merged.

Yep, that makes sense.

Jean-Paul

[Jean-Paul Calderone]

On Sun, Jan 22, 2017 at 8:31 PM, Jean-Paul Calderone <

Post by Jean-Paul Calderone

On Jan 22, 2017, at 5:24 PM, Jean-Paul Calderone <

On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone <
Hello,
I didn't find any hints about the workflow surrounding the
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76
Jean-Paul
Thanks for writing this up. The workflow is evolving organically.
However, the "close the PR" step confuses me. If you do this, the
original contributor won't be able to respond to feedback. What is the
desired effect of this second PR?

The idea I heard is that it provides a place to hang build
failure-related review comments. I also had some vague notion that it
would be the place you'd look to see the complete CI results.
So. Where should further reviews go and where do you find CI results, if
you don't create a new PR?
You find the CI results on the existing PR; they'll be displayed there.
You just need to make sure that a branch in the repo has the exact same
commit ID when you push it as the PR has. (Statuses are reported on
commits, and triggered on pushes, so everything should just line up.)

After the branch in the repo is all set (which I assume pr_as_branch will
do), does CI notice without further help and start the necessary builds?

As far as further reviews - on the existing PR too, same as usual.
You will need to run the script again when the contributor pushes their
response to a review. One feature that would be useful is to not need to
specify the branch name twice, but if you name the second pr_as_branch
invocation differently, it just means one more branch that should be
deleted when looking at git branch --merged.

Yep, that makes sense.

Updated:
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=77&old_version=75

Post by Jean-Paul Calderone
Jean-Paul

[Glyph Lefkowitz]

Post by Glyph Lefkowitz

Post by Glyph Lefkowitz

Post by Jean-Paul Calderone
Hello,
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76 <https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76>
Jean-Paul

Thanks for writing this up. The workflow is evolving organically.
However, the "close the PR" step confuses me. If you do this, the original contributor won't be able to respond to feedback. What is the desired effect of this second PR?
The idea I heard is that it provides a place to hang build failure-related review comments. I also had some vague notion that it would be the place you'd look to see the complete CI results.
So. Where should further reviews go and where do you find CI results, if you don't create a new PR?

You find the CI results on the existing PR; they'll be displayed there. You just need to make sure that a branch in the repo has the exact same commit ID when you push it as the PR has. (Statuses are reported on commits, and triggered on pushes, so everything should just line up.)
After the branch in the repo is all set (which I assume pr_as_branch will do), does CI notice without further help and start the necessary builds?
As far as further reviews - on the existing PR too, same as usual.
You will need to run the script again when the contributor pushes their response to a review. One feature that would be useful is to not need to specify the branch name twice, but if you name the second pr_as_branch invocation differently, it just means one more branch that should be deleted when looking at git branch --merged.
Yep, that makes sense.
Updated: https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=77&old_version=75 <https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=77&old_version=75>

Thanks!

-g

[Glyph Lefkowitz]

Post by Glyph Lefkowitz

Post by Glyph Lefkowitz

Post by Jean-Paul Calderone
Hello,
https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76 <https://twistedmatrix.com/trac/wiki/ReviewProcess?action=diff&version=76>
Jean-Paul

Thanks for writing this up. The workflow is evolving organically.
However, the "close the PR" step confuses me. If you do this, the original contributor won't be able to respond to feedback. What is the desired effect of this second PR?
The idea I heard is that it provides a place to hang build failure-related review comments. I also had some vague notion that it would be the place you'd look to see the complete CI results.
So. Where should further reviews go and where do you find CI results, if you don't create a new PR?

You find the CI results on the existing PR; they'll be displayed there. You just need to make sure that a branch in the repo has the exact same commit ID when you push it as the PR has. (Statuses are reported on commits, and triggered on pushes, so everything should just line up.)
After the branch in the repo is all set (which I assume pr_as_branch will do), does CI notice without further help and start the necessary builds?

It should, yes. It's buildbot that notices, and (my understanding is) the hook buildbot uses is "push" not "new PR". It runs for branches in the repo even if they don't have a PR yet.

Post by Glyph Lefkowitz
As far as further reviews - on the existing PR too, same as usual.
You will need to run the script again when the contributor pushes their response to a review. One feature that would be useful is to not need to specify the branch name twice, but if you name the second pr_as_branch invocation differently, it just means one more branch that should be deleted when looking at git branch --merged.
Yep, that makes sense.
Jean-Paul
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python