Mailing List Archive

What going on in Qauuag repo?
Just browsed the repo and there is a truck load of changes there.
Some are big "merges" which changes stuff all over the place in one commmit,
see http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=14d2bbaa3f4aa53152472694c29f336808e47313
(Merge quagga mainline into the google ISIS code.) for example

Then there are commits which are sane but I have not
seen them on the ML or any other discussion.
Is this the new maintainer model or some accident?

Jocke

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
Re: What going on in Qauuag repo? [ In reply to ]
On 18/04/2012 18:29, Joakim Tjernlund wrote:
> Just browsed the repo and there is a truck load of changes there.
> Some are big "merges" which changes stuff all over the place in one commmit,
> see http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=14d2bbaa3f4aa53152472694c29f336808e47313
> (Merge quagga mainline into the google ISIS code.) for example
>
> Then there are commits which are sane but I have not
> seen them on the ML or any other discussion.
> Is this the new maintainer model or some accident?

This commit seems to be an amalgamation of some minor fixed and code
tweaks, some work relevant the the netlink layer, and a massive chunk of
ISIS fixed. Probably the minor fixes and the netlink stuff should have
been separated out, but I'm ok about the idea of the quagga maintainers
committing the isis code in bulk: the previous code was seriously broken.
From what I understand, the new code is significantly less so.

Nick
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
Re: What going on in Qauuag repo? [ In reply to ]
Nick Hilliard <nick@inex.ie> wrote on 2012/04/18 18:58:39:

> From: Nick Hilliard <nick@inex.ie>
> To: Quagga development list <quagga-dev@lists.quagga.net>
> Cc: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> Date: 2012/04/18 18:58
> Subject: Re: [quagga-dev 9197] What going on in Qauuag repo?
>
> On 18/04/2012 18:29, Joakim Tjernlund wrote:
> > Just browsed the repo and there is a truck load of changes there.
> > Some are big "merges" which changes stuff all over the place in one commmit,
> > see http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=14d2bbaa3f4aa53152472694c29f336808e47313
> > (Merge quagga mainline into the google ISIS code.) for example
> >
> > Then there are commits which are sane but I have not
> > seen them on the ML or any other discussion.
> > Is this the new maintainer model or some accident?
>
> This commit seems to be an amalgamation of some minor fixed and code
> tweaks, some work relevant the the netlink layer, and a massive chunk of
> ISIS fixed. Probably the minor fixes and the netlink stuff should have
> been separated out, but I'm ok about the idea of the quagga maintainers
> committing the isis code in bulk: the previous code was seriously broken.
> From what I understand, the new code is significantly less so.

Yes I quoted the wrong commit, sorry.
It should be http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=3cf6c2b4e43f44a977d218c96c26250654ae333e
Merge branch 'quagga' into google-bgp-multipath

Jocke

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
Re: What going on in Qauuag repo? [ In reply to ]
Joakim Tjernlund/Transmode wrote on 2012/04/18 21:30:55:
>
> Nick Hilliard <nick@inex.ie> wrote on 2012/04/18 18:58:39:
>
> > From: Nick Hilliard <nick@inex.ie>
> > To: Quagga development list <quagga-dev@lists.quagga.net>
> > Cc: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> > Date: 2012/04/18 18:58
> > Subject: Re: [quagga-dev 9197] What going on in Qauuag repo?
> >
> > On 18/04/2012 18:29, Joakim Tjernlund wrote:
> > > Just browsed the repo and there is a truck load of changes there.
> > > Some are big "merges" which changes stuff all over the place in one commmit,
> > > see http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=14d2bbaa3f4aa53152472694c29f336808e47313
> > > (Merge quagga mainline into the google ISIS code.) for example
> > >
> > > Then there are commits which are sane but I have not
> > > seen them on the ML or any other discussion.
> > > Is this the new maintainer model or some accident?
> >
> > This commit seems to be an amalgamation of some minor fixed and code
> > tweaks, some work relevant the the netlink layer, and a massive chunk of
> > ISIS fixed. Probably the minor fixes and the netlink stuff should have
> > been separated out, but I'm ok about the idea of the quagga maintainers
> > committing the isis code in bulk: the previous code was seriously broken.
> > From what I understand, the new code is significantly less so.

> Yes I quoted the wrong commit, sorry.
> It should be http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=3cf6c2b4e43f44a977d218c96c26250654ae333e
> Merge branch 'quagga' into google-bgp-multipath

Ahh, the cgit web interface is broken, it shows the wrong diff!
Could the Quagga web point to the other, native to git, web I/F instead:
http://git.savannah.gnu.org/gitweb/?p=quagga.git


_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
Re: What going on in Qauuag repo? [ In reply to ]
Hi Joakim,


you're kinda preemting an announcement mail that I have in my outbound
queue ;) - well.

On Wed, Apr 18, 2012 at 09:51:12PM +0200, Joakim Tjernlund wrote:
> > > > Then there are commits which are sane but I have not
> > > > seen them on the ML or any other discussion.
> > > > Is this the new maintainer model or some accident?

I've merged 2 branches and 1 single-commit; both of the branches have
(old) threads on the ML. I'm not aware of any negative feedback on
these patches that hasn't been discussed (the BGP multipath stuff had
some discussion, but in the end it seemed OK to merge).

A new maintainer model - well, I didn't feel the need to rediscuss these
branches, so, I guess I'm a bit more aggressive. We discussed this with
the maintainers and we think the inactivity and staleness of the master
branch is doing way more harm than more aggressive merging and maybe
having a broken patch once in a while.

(I did indeed make a mistake on the merges, with af56d40, which is a
duplicate of 2ea1ab1 - I'll revert one of them as soon as I figure which
is better.)

The ISIS code submitted by Google was cleaned up by the OpenSouceRouting
people, in particular Avneesh Sachdev. He did a good job isolating out
all the non-isis changes so we have at least some transparency and
reviewability. About the remaining 2 bulk isisd changesets... TBFH I'd
very much welcome a complete rewrite, as broken as isisd is. The policy
on isisd is basically "accept everything".

On the other side, I'm currently looking at Euro-IX and Vyatta. The
latter I might be able to pick apart, but the former might end up being
merged in one giant batch; I'm nowhere near close to figuring out what
to do there. The only thing I'm sure on is that it _must_ be merged;
that came out pretty clear based off the feedback I've received at
RIPE64 from users.

> > > This commit seems to be an amalgamation of some minor fixed and code
> > > tweaks, some work relevant the the netlink layer, and a massive chunk of
> > > ISIS fixed. Probably the minor fixes and the netlink stuff should have
> > > been separated out, but I'm ok about the idea of the quagga maintainers
> > > committing the isis code in bulk: the previous code was seriously broken.
> > > From what I understand, the new code is significantly less so.
>
> > Yes I quoted the wrong commit, sorry.
> > It should be http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=3cf6c2b4e43f44a977d218c96c26250654ae333e
> > Merge branch 'quagga' into google-bgp-multipath
>
> Ahh, the cgit web interface is broken, it shows the wrong diff!
> Could the Quagga web point to the other, native to git, web I/F instead:
> http://git.savannah.gnu.org/gitweb/?p=quagga.git

Hm, it came up with an error for me. I guess this is the one:
http://git.savannah.gnu.org/gitweb/?p=quagga.git;a=commit;h=3cf6c2b4e43f44a977d218c96c26250654ae333e
(No clue about savannah mechanics here btw.)

The one you linked before (3cf6c2b) was a preparatory merge that Avneesh
made on his tree.

Cheers,

-David
Re: What going on in Qauuag repo? [ In reply to ]
David Lamparter <equinox@diac24.net> wrote on 2012/04/19 10:38:55:
>
> Hi Joakim,
>
>
> you're kinda preemting an announcement mail that I have in my outbound
> queue ;) - well.

Oops, sorry :)

>
> On Wed, Apr 18, 2012 at 09:51:12PM +0200, Joakim Tjernlund wrote:
> > > > > Then there are commits which are sane but I have not
> > > > > seen them on the ML or any other discussion.
> > > > > Is this the new maintainer model or some accident?
>
> I've merged 2 branches and 1 single-commit; both of the branches have
> (old) threads on the ML. I'm not aware of any negative feedback on
> these patches that hasn't been discussed (the BGP multipath stuff had
> some discussion, but in the end it seemed OK to merge).
>
> A new maintainer model - well, I didn't feel the need to rediscuss these
> branches, so, I guess I'm a bit more aggressive. We discussed this with
> the maintainers and we think the inactivity and staleness of the master
> branch is doing way more harm than more aggressive merging and maybe
> having a broken patch once in a while.
>
> (I did indeed make a mistake on the merges, with af56d40, which is a
> duplicate of 2ea1ab1 - I'll revert one of them as soon as I figure which
> is better.)
>
> The ISIS code submitted by Google was cleaned up by the OpenSouceRouting
> people, in particular Avneesh Sachdev. He did a good job isolating out
> all the non-isis changes so we have at least some transparency and
> reviewability. About the remaining 2 bulk isisd changesets... TBFH I'd
> very much welcome a complete rewrite, as broken as isisd is. The policy
> on isisd is basically "accept everything".
>
> On the other side, I'm currently looking at Euro-IX and Vyatta. The
> latter I might be able to pick apart, but the former might end up being
> merged in one giant batch; I'm nowhere near close to figuring out what
> to do there. The only thing I'm sure on is that it _must_ be merged;
> that came out pretty clear based off the feedback I've received at
> RIPE64 from users.

I relize that there are lots stuff in other repos that are hard to break up
in pieces so there will be some bulk merges and I can live with that. Quagga
needs to be brought up to speed sooner than later.
However there are some 25+ commits the last two weeks and it ain't all merges,
yet very little was sent out on the dev. ML as patches for all to see/comment
on.
If you want the Q. community to help you looking at patches, they must go out on the
ML as inline patches before you commit them.

>
> > > > This commit seems to be an amalgamation of some minor fixed and code
> > > > tweaks, some work relevant the the netlink layer, and a massive chunk of
> > > > ISIS fixed. Probably the minor fixes and the netlink stuff should have
> > > > been separated out, but I'm ok about the idea of the quagga maintainers
> > > > committing the isis code in bulk: the previous code was seriously broken.
> > > > From what I understand, the new code is significantly less so.
> >
> > > Yes I quoted the wrong commit, sorry.
> > > It should be http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=3cf6c2b4e43f44a977d218c96c26250654ae333e
> > > Merge branch 'quagga' into google-bgp-multipath
> >
> > Ahh, the cgit web interface is broken, it shows the wrong diff!
> > Could the Quagga web point to the other, native to git, web I/F instead:
> > http://git.savannah.gnu.org/gitweb/?p=quagga.git
>
> Hm, it came up with an error for me. I guess this is the one:
> http://git.savannah.gnu.org/gitweb/?p=quagga.git;a=commit;h=3cf6c2b4e43f44a977d218c96c26250654ae333e
> (No clue about savannah mechanics here btw.)

hmm, works for me. Does
http://git.savannah.gnu.org/gitweb/?p=quagga.git;a=summary
work any better for you?

>
> The one you linked before (3cf6c2b) was a preparatory merge that Avneesh
> made on his tree.

hmm, look like Avneesh committed his stuff himself? He is listed as committer.

Jocke

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
Re: What going on in Qauuag repo? [ In reply to ]
On Thu, Apr 19, 2012 at 01:23:30PM +0200, Joakim Tjernlund wrote:
> I relize that there are lots stuff in other repos that are hard to break up
> in pieces so there will be some bulk merges and I can live with that. Quagga
> needs to be brought up to speed sooner than later.
> However there are some 25+ commits the last two weeks and it ain't all merges,
> yet very little was sent out on the dev. ML as patches for all to see/comment
> on.
> If you want the Q. community to help you looking at patches, they must go out on the
> ML as inline patches before you commit them.

Well, yeah, that's why I said they were posted some time ago... I mean,
we can't go through all the patches 5 times before merging. The BGP
multipath had quite some discussion, in the end Paul gave it an ACK, and
since he was somewhat opposed to it initially I reckoned that works out.

The ISIS stuff could've used more review I guess, especially since the
split-up version wasn't available beforehand.

Generally, I agree with you. There are problems when a maintainer
directly commits to the git without involving the mailing list.
I've received and understood your concerns and will address them!

Btw, I'd welcome suggestions on how to handle Euro-IX and Vyatta - the
latter is available at http://git.vyatta.com/vyatta-quagga.git so
everyone can look at it. (that's the git url, for http/gitweb it's
https://bugzilla.vyatta.com/git/ I think)

> > The one you linked before (3cf6c2b) was a preparatory merge that Avneesh
> > made on his tree.
>
> hmm, look like Avneesh committed his stuff himself? He is listed as committer.

Avneesh committed it to the OSR git, which I merged. I guess I should
stop editing the description of merge commits... it kinda makes them
non-obvious in a text view. I use qgit & giggle a lot, so I wasn't
aware of that.


-David
Re: What going on in Qauuag repo? [ In reply to ]
David Lamparter <equinox@diac24.net> wrote on 2012/04/19 13:39:56:
>
> On Thu, Apr 19, 2012 at 01:23:30PM +0200, Joakim Tjernlund wrote:
> > I relize that there are lots stuff in other repos that are hard to break up
> > in pieces so there will be some bulk merges and I can live with that. Quagga
> > needs to be brought up to speed sooner than later.
> > However there are some 25+ commits the last two weeks and it ain't all merges,
> > yet very little was sent out on the dev. ML as patches for all to see/comment
> > on.
> > If you want the Q. community to help you looking at patches, they must go out on the
> > ML as inline patches before you commit them.
>
> Well, yeah, that's why I said they were posted some time ago... I mean,
> we can't go through all the patches 5 times before merging. The BGP

Sure, I guess those patches are really old ones that already been at the list.

> multipath had quite some discussion, in the end Paul gave it an ACK, and
> since he was somewhat opposed to it initially I reckoned that works out.
>
> The ISIS stuff could've used more review I guess, especially since the
> split-up version wasn't available beforehand.
>
> Generally, I agree with you. There are problems when a maintainer
> directly commits to the git without involving the mailing list.
> I've received and understood your concerns and will address them!
>
> Btw, I'd welcome suggestions on how to handle Euro-IX and Vyatta - the
> latter is available at http://git.vyatta.com/vyatta-quagga.git so
> everyone can look at it. (that's the git url, for http/gitweb it's
> https://bugzilla.vyatta.com/git/ I think)

I looked a little via gitweb and it is hard to make out what to do. There are several
branches which means nothing to me and most commits are from outside Vyatta.

It might be easier to look for the vyatta patches Stephen sent to the list? They must
be archived somewhere.

Jocke

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
Re: What going on in Qauuag repo? [ In reply to ]
On Thu, Apr 19, 2012 at 04:47:32PM +0200, Joakim Tjernlund wrote:
> > Btw, I'd welcome suggestions on how to handle Euro-IX and Vyatta - the
> > latter is available at http://git.vyatta.com/vyatta-quagga.git so
> > everyone can look at it. (that's the git url, for http/gitweb it's
> > https://bugzilla.vyatta.com/git/ I think)
>
> I looked a little via gitweb and it is hard to make out what to do. There are several
> branches which means nothing to me and most commits are from outside Vyatta.
>
> It might be easier to look for the vyatta patches Stephen sent to the list? They must
> be archived somewhere.

Actually, looking at the diff between
6884fa3 Merge branch 'quagga_0.99.20.1' into pacifica
87f81ea release: 0.99.20.1
it looks Okay-ish:
107 files changed, 4767 insertions(+), 1288 deletions(-)
minus the debian/ directory, which is
21 files changed, 2998 insertions(+), 0 deletions(-)
that leaves
86 files changed, 1769 insertions(+), 1288 deletions(-)
which is quite okay I guess...

I think it might be easier just picking off patches going from the diff
with git blame... (or, review 1800 lines of diff, it's not THAT much)

Stephen, do you have any comments?


-David
Re: What going on in Qauuag repo? [ In reply to ]
On Thu, 19 Apr 2012 17:05:21 +0200
David Lamparter <equinox@diac24.net> wrote:

> On Thu, Apr 19, 2012 at 04:47:32PM +0200, Joakim Tjernlund wrote:
> > > Btw, I'd welcome suggestions on how to handle Euro-IX and Vyatta - the
> > > latter is available at http://git.vyatta.com/vyatta-quagga.git so
> > > everyone can look at it. (that's the git url, for http/gitweb it's
> > > https://bugzilla.vyatta.com/git/ I think)
> >
> > I looked a little via gitweb and it is hard to make out what to do. There are several
> > branches which means nothing to me and most commits are from outside Vyatta.
> >
> > It might be easier to look for the vyatta patches Stephen sent to the list? They must
> > be archived somewhere.
>
> Actually, looking at the diff between
> 6884fa3 Merge branch 'quagga_0.99.20.1' into pacifica
> 87f81ea release: 0.99.20.1
> it looks Okay-ish:
> 107 files changed, 4767 insertions(+), 1288 deletions(-)
> minus the debian/ directory, which is
> 21 files changed, 2998 insertions(+), 0 deletions(-)
> that leaves
> 86 files changed, 1769 insertions(+), 1288 deletions(-)
> which is quite okay I guess...
>
> I think it might be easier just picking off patches going from the diff
> with git blame... (or, review 1800 lines of diff, it's not THAT much)

Better to look at the diff, there were many cases where the solution
had to evolve (like recursive routes), and you probably don't want
to look at the intermediate steps.
Re: What going on in Qauuag repo? [ In reply to ]
On Wed, 18 Apr 2012, Joakim Tjernlund wrote:

> Ahh, the cgit web interface is broken, it shows the wrong diff!
> Could the Quagga web point to the other, native to git, web I/F instead:
> http://git.savannah.gnu.org/gitweb/?p=quagga.git

Ah, the Savannah project page already does:

http://savannah.nongnu.org/git/?group=quagga

Oops on the link on the web page. ;)

regards,
--
Paul Jakma paul@jakma.org @pjakma Key ID: 64A2FF6A
Fortune:
Time sure flies when you don't know what you're doing.
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
Re: What going on in Qauuag repo? [ In reply to ]
On Thu, 19 Apr 2012, David Lamparter wrote:

> to do there. The only thing I'm sure on is that it _must_ be merged;
> that came out pretty clear based off the feedback I've received at
> RIPE64 from users.

I think it would need a discussion here first, tbh. There are other,
multi-process models for BGP.

regards,
--
Paul Jakma paul@jakma.org @pjakma Key ID: 64A2FF6A
Fortune:
Lack of capability is usually disguised by lack of interest.
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev