trac diffs

[R. Andrew Ohana]

Hey Everyone,

As I've been working on the dev scripts, I've been seeing how trac has been dealing with them, and have found that I've needed to make some small changes here and there to the git functionality to get what we want. See github.com/ohanar/trac to see what is currently running at trac.tangentspace.org

Using merge-bases for the ticket branch plugin was also wrong. Consider (the diagram needs to be viewed in a monospace font):

mainline: A - B - C - D
           \       \
feature:    E - F - G - H

The merge base for H and D is C, but what we wanted was A, so we can review E, F, G, and H. I've already implemented this change in the ticket branch plugin, but it has an annoying side effect: the default "View Changes" diff is now from A..H, which will include all the changes that have been merged into the mainline branch as well. I've attached the dev_scripts branch to trac.tangentspace.org/14475, so you can see what I mean.

I suppose that a solution would be to add an extra button (maybe in the branch field) that shows an overall diff, but let me know if you have any other ideas on how to resolve this.

--

Andrew

[Robert Bradshaw]

Wouldn't it be better to view the changes D..H by default? I guess I'm
not seeing why this change is what we "wanted."

> --
> You received this message because you are subscribed to the Google Groups
> "sage-git" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sage-git+u...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[R. Andrew Ohana]

D..H is not what we want either, that would include the diff for D..C which is irrelevant to the feature branch. What we really want is the diff H..I where I is the merge of D and H.

What we want is to be able to see all the commits that are introduced by the feature branch, and nothing more. By using the merge base, we won't get all of the commits if master was re-merged into the feature branch, which is why the change was needed (when the plugin was configured to use the merge base, the log would only display G and H, while E and F were completely hidden from every easily accessible view).

--

Andrew

[Keshav Kini]

On Thu, May 9, 2013 at 10:50 PM, R. Andrew Ohana <andrew...@gmail.com> wrote:
> D..H is not what we want either, that would include the diff for D..C which
> is irrelevant to the feature branch. What we really want is the diff H..I
> where I is the merge of D and H.

If I'm not mistaken, what we really want (according to the
"dependencies" stuff we were talking about in Portland) is the diff
between X and Y, where X is the merge of master and all the marked
dependencies of the ticket, and Y is the merge of X with the ticket
head. I believe we also discussed caching these commits X in the trac
server plugin so as to not need to run `git merge` twice on every page
load.

-Keshav

[Robert Bradshaw]

On Thu, May 9, 2013 at 10:50 PM, R. Andrew Ohana <andrew...@gmail.com> wrote:
> D..H is not what we want either,

I meant C..H.

> that would include the diff for D..C which
> is irrelevant to the feature branch. What we really want is the diff H..I
> where I is the merge of D and H.

Did you mean D..I?

> What we want is to be able to see all the commits that are introduced by the
> feature branch, and nothing more. By using the merge base, we won't get all
> of the commits if master was re-merged into the feature branch, which is why
> the change was needed (when the plugin was configured to use the merge base,
> the log would only display G and H, while E and F were completely hidden
> from every easily accessible view).

We should have a link to the diff C..H as well as a link to all
commits in H not in D (or C, it doesn't matter). A..H seems really bad
though. How to do this I'm not sure...

[R. Andrew Ohana]

On Thu, May 9, 2013 at 11:11 PM, Robert Bradshaw <robe...@gmail.com> wrote:

On Thu, May 9, 2013 at 10:50 PM, R. Andrew Ohana <andrew...@gmail.com> wrote:
> D..H is not what we want either,

I meant C..H.

For the dev scripts C..H would be
http://trac.tangentspace.org/changeset?reponame=&new=5c6f0b3d20ce1319b3153a3c4de0fd9c758e6bff%40%2F&old=c39e011c881b6bfa20ec3c0670bc2b62b11a752e%40%2F

To me it is pretty obvious that this should not be the default diff (as it is showing changes within the dev scripts which don't even exist in master).

> that would include the diff for D..C which
> is irrelevant to the feature branch. What we really want is the diff H..I
> where I is the merge of D and H.

Did you mean D..I?

Yes, and in the case of dependencies, we should do what Keshav suggests.

> What we want is to be able to see all the commits that are introduced by the
> feature branch, and nothing more. By using the merge base, we won't get all
> of the commits if master was re-merged into the feature branch, which is why
> the change was needed (when the plugin was configured to use the merge base,
> the log would only display G and H, while E and F were completely hidden
> from every easily accessible view).

We should have a link to the diff C..H as well as a link to all
commits in H not in D (or C, it doesn't matter). A..H seems really bad
though. How to do this I'm not sure...

It is easy enough to get the diff between any two commits that are in history (i.e. not I, X, or Y) by just selecting the appropriate bubbles in the log and clicking "View Changes".

--

Andrew