Re: Doing chained diffs w/ Reviewboard

2014-09-18 Thread Adam Collard
On 18 September 2014 10:49, John Meinel j...@arbash-meinel.com wrote:

 Has anyone succeeded in getting this to work?

 The steps I tried to do were:

  git co master
  git pull upstream master
  git co base-branch
  git diff master...  base.diff
  git co dependent-branch
  git diff master...  dependent.diff
  git merge-base master HEAD  remember-this-rev

 And then put the dependent.diff into the Diff: *, and then the
 base.diff into Parent Diff: and then 'remember-this-rev' into the Base
 Commit ID.

 I also tried putting git merge-base master base-branch as the Base
 Commit ID.


This makes me think you're using the UI to do this.

Let me repeat my Public Safety Announcement: Do NOT use ReviewBoard's UI
for uploading diffs. Please for $deity's sake use rbt post.

https://www.reviewboard.org/docs/rbtools/0.6/rbt/commands/post/#distributed-version-control-systems
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Doing chained diffs w/ Reviewboard

2014-09-18 Thread Matthew Williams
I've got it working. Using rbt it was pretty trivial. I'm not 100% sure of
my steps - but from memory and some prompting from `history` the process
was more or less:

1) rebase my branch against the latest version of the parent. Then:

2) rbt post -parent remotes/mattyw/my-parent-branch

It appeared that when I ran rbt -u against this branch I still needed to
specify the parent (ISTR that lbox used to remember there was a parent
branch when pushing changes)

Matty



On Thu, Sep 18, 2014 at 10:53 AM, Adam Collard adam.coll...@canonical.com
wrote:

 On 18 September 2014 10:49, John Meinel j...@arbash-meinel.com wrote:

 Has anyone succeeded in getting this to work?

 The steps I tried to do were:

  git co master
  git pull upstream master
  git co base-branch
  git diff master...  base.diff
  git co dependent-branch
  git diff master...  dependent.diff
  git merge-base master HEAD  remember-this-rev

 And then put the dependent.diff into the Diff: *, and then the
 base.diff into Parent Diff: and then 'remember-this-rev' into the Base
 Commit ID.

 I also tried putting git merge-base master base-branch as the Base
 Commit ID.


 This makes me think you're using the UI to do this.

 Let me repeat my Public Safety Announcement: Do NOT use ReviewBoard's UI
 for uploading diffs. Please for $deity's sake use rbt post.


 https://www.reviewboard.org/docs/rbtools/0.6/rbt/commands/post/#distributed-version-control-systems

 --
 Juju-dev mailing list
 Juju-dev@lists.ubuntu.com
 Modify settings or unsubscribe at:
 https://lists.ubuntu.com/mailman/listinfo/juju-dev


-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Doing chained diffs w/ Reviewboard

2014-09-18 Thread John Meinel
So I did get rbt post to work with rbt post -r 54 --parent=REVID, I had
to be careful because my actual parent branch *didn't* merge the current
tip of master but the one I was proposing had.
So instead I ended up rebasing both commits, and then specifying the PARENT
as the specific rebased commit revision id.

Launchpad had a place to record a prerequisite branch, which I'm sure is
what lbox was using. Launchpad also computed the diff by starting with
master merging the prerequisite branch into it, and then your diff was
the diff of merging your branch into the combined diff. Which meant it
handled all these side cases where your branch merges more of master than
your parent branch, etc.

John
=:-

On Thu, Sep 18, 2014 at 2:28 PM, Matthew Williams 
matthew.willi...@canonical.com wrote:

 I've got it working. Using rbt it was pretty trivial. I'm not 100% sure of
 my steps - but from memory and some prompting from `history` the process
 was more or less:

 1) rebase my branch against the latest version of the parent. Then:

 2) rbt post -parent remotes/mattyw/my-parent-branch

 It appeared that when I ran rbt -u against this branch I still needed to
 specify the parent (ISTR that lbox used to remember there was a parent
 branch when pushing changes)

 Matty



 On Thu, Sep 18, 2014 at 10:53 AM, Adam Collard adam.coll...@canonical.com
  wrote:

 On 18 September 2014 10:49, John Meinel j...@arbash-meinel.com wrote:

 Has anyone succeeded in getting this to work?

 The steps I tried to do were:

  git co master
  git pull upstream master
  git co base-branch
  git diff master...  base.diff
  git co dependent-branch
  git diff master...  dependent.diff
  git merge-base master HEAD  remember-this-rev

 And then put the dependent.diff into the Diff: *, and then the
 base.diff into Parent Diff: and then 'remember-this-rev' into the Base
 Commit ID.

 I also tried putting git merge-base master base-branch as the Base
 Commit ID.


 This makes me think you're using the UI to do this.

 Let me repeat my Public Safety Announcement: Do NOT use ReviewBoard's UI
 for uploading diffs. Please for $deity's sake use rbt post.


 https://www.reviewboard.org/docs/rbtools/0.6/rbt/commands/post/#distributed-version-control-systems

 --
 Juju-dev mailing list
 Juju-dev@lists.ubuntu.com
 Modify settings or unsubscribe at:
 https://lists.ubuntu.com/mailman/listinfo/juju-dev



-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Doing chained diffs w/ Reviewboard

2014-09-18 Thread John Meinel
Certainly git can do the same try a merge of branch A into master, and
then try a merge of branch B into that and use that as the diff. And if
rbt can take just a diff to post, then we should be able to publish that.
But I don't know where the data of *what* branches are involved would be
stored, as Reviewboard seems to just think in terms of diffs. (rbt -u just
guesses what review is associated to your branch by looking at the summary
messages, there is no direct link to a branch AFAICT)

John
=:-

On Thu, Sep 18, 2014 at 2:45 PM, Ian Booth ian.bo...@canonical.com wrote:



 On 18/09/14 20:39, John Meinel wrote:
  So I did get rbt post to work with rbt post -r 54 --parent=REVID, I
 had
  to be careful because my actual parent branch *didn't* merge the current
  tip of master but the one I was proposing had.
  So instead I ended up rebasing both commits, and then specifying the
 PARENT
  as the specific rebased commit revision id.
 
  Launchpad had a place to record a prerequisite branch, which I'm sure is
  what lbox was using. Launchpad also computed the diff by starting with
  master merging the prerequisite branch into it, and then your diff was
  the diff of merging your branch into the combined diff. Which meant it
  handled all these side cases where your branch merges more of master than
  your parent branch, etc.
 

 And that, ladies and gentlemen, is why I *really* miss Launchpad. Doing
 what
 John did sounds error prone and I'm sure I'd mess it up if I tried.

 Eric, can we do something on the review board and/or github side to make
 this
 sort of thing easier? Implement a script which follows the Launchpad
 workflow or
 something?


-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Doing chained diffs w/ Reviewboard

2014-09-18 Thread David Cheney
+1 to that

On Thu, Sep 18, 2014 at 7:53 PM, Adam Collard
adam.coll...@canonical.com wrote:
 On 18 September 2014 10:49, John Meinel j...@arbash-meinel.com wrote:

 Has anyone succeeded in getting this to work?

 The steps I tried to do were:

  git co master
  git pull upstream master
  git co base-branch
  git diff master...  base.diff
  git co dependent-branch
  git diff master...  dependent.diff
  git merge-base master HEAD  remember-this-rev

 And then put the dependent.diff into the Diff: *, and then the
 base.diff into Parent Diff: and then 'remember-this-rev' into the Base
 Commit ID.

 I also tried putting git merge-base master base-branch as the Base
 Commit ID.


 This makes me think you're using the UI to do this.

 Let me repeat my Public Safety Announcement: Do NOT use ReviewBoard's UI for
 uploading diffs. Please for $deity's sake use rbt post.

 https://www.reviewboard.org/docs/rbtools/0.6/rbt/commands/post/#distributed-version-control-systems

 --
 Juju-dev mailing list
 Juju-dev@lists.ubuntu.com
 Modify settings or unsubscribe at:
 https://lists.ubuntu.com/mailman/listinfo/juju-dev


-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev


Re: Doing chained diffs w/ Reviewboard

2014-09-18 Thread David Cheney
Also, be watchful for the other reviewboard footgun, paged diffs.

Reviewboard pages large reviews, so if you're used to thinking 'phew,
i've gotten to the end of the page, i'm done, check again, there
maybe a surprise waiting for you at the bottom of the page.

On Thu, Sep 18, 2014 at 9:03 PM, David Cheney
david.che...@canonical.com wrote:
 +1 to that

 On Thu, Sep 18, 2014 at 7:53 PM, Adam Collard
 adam.coll...@canonical.com wrote:
 On 18 September 2014 10:49, John Meinel j...@arbash-meinel.com wrote:

 Has anyone succeeded in getting this to work?

 The steps I tried to do were:

  git co master
  git pull upstream master
  git co base-branch
  git diff master...  base.diff
  git co dependent-branch
  git diff master...  dependent.diff
  git merge-base master HEAD  remember-this-rev

 And then put the dependent.diff into the Diff: *, and then the
 base.diff into Parent Diff: and then 'remember-this-rev' into the Base
 Commit ID.

 I also tried putting git merge-base master base-branch as the Base
 Commit ID.


 This makes me think you're using the UI to do this.

 Let me repeat my Public Safety Announcement: Do NOT use ReviewBoard's UI for
 uploading diffs. Please for $deity's sake use rbt post.

 https://www.reviewboard.org/docs/rbtools/0.6/rbt/commands/post/#distributed-version-control-systems

 --
 Juju-dev mailing list
 Juju-dev@lists.ubuntu.com
 Modify settings or unsubscribe at:
 https://lists.ubuntu.com/mailman/listinfo/juju-dev


-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev