[Twisted-Python] admin/pr_as_branch
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
On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
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 <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
On Sun, Jan 22, 2017 at 8:19 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
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
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 Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On Jan 22, 2017, at 5:24 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Sun, Jan 22, 2017 at 8:19 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
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 <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
On Sun, Jan 22, 2017 at 8:28 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Jan 22, 2017, at 5:24 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
On Sun, Jan 22, 2017 at 8:19 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
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
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
On Sun, Jan 22, 2017 at 8:31 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
On Sun, Jan 22, 2017 at 8:28 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Jan 22, 2017, at 5:24 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
On Sun, Jan 22, 2017 at 8:19 PM, Glyph Lefkowitz <glyph@twistedmatrix.com
wrote:
On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
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
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
Jean-Paul
On Jan 22, 2017, at 5:51 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Sun, Jan 22, 2017 at 8:31 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote: On Sun, Jan 22, 2017 at 8:28 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Jan 22, 2017, at 5:24 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
On Sun, Jan 22, 2017 at 8:19 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
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 <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
On Jan 22, 2017, at 5:31 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Sun, Jan 22, 2017 at 8:28 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Jan 22, 2017, at 5:24 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
On Sun, Jan 22, 2017 at 8:19 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Jan 22, 2017, at 5:15 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
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 <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.
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 Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
participants (2)
-
Glyph Lefkowitz -
Jean-Paul Calderone