Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wingo-next #732

Merged
merged 2 commits into from
Feb 3, 2016
Merged

Add wingo-next #732

merged 2 commits into from
Feb 3, 2016

Conversation

wingo
Copy link
Contributor

@wingo wingo commented Feb 2, 2016

Following #725, this PR adds a wingo-next branch to branches.md. We'll set up the automatic review assignment process shortly. Thanks for the encouragement @lukego, I look forward to your and @eugeneia's help as we fumble towards a distributed branch setup :)

@lukego
Copy link
Member

lukego commented Feb 2, 2016

Great! Awesome! Fantastic!

If I read this correctly then the proposal is for wingo-next to feed upstream to next and therefore run in parallel to max-next giving us this graph:

graph

I am happy for the next branch to be upstream for both max-next and wingo-next and to work with you guys to make this workflow produce good results that everybody is happy with.

First request: how do we define the "downstream" branches that feed into max-next and wingo-next? If both branches will be processing PRs sent to the main Github repo then what is the "sharding function" ("routing table") for unambiguously deciding whether a given change is for max-next or wingo-next?

The Linux kernel has the MAINTAINERS file that defines the "first hop" for new incoming changes. We would seem to need some equivalent. I would like contributors to always know what is their next-hop and I would prefer not to pull the same changes from both max-next and wingo-next.

One suggestion would be to choose a more specific subject area for one or both of the branches, and rename the branch to reflect the changes that it accepts, but any solution that is unambiguous and agreeable to everybody should be fine.

@wingo
Copy link
Contributor Author

wingo commented Feb 2, 2016

Hi :)

First request: how do we define the "downstream" branches that feed into max-next and wingo-next?

Let's list this in branches.md.

If both branches will be processing PRs sent to the main Github repo then what is the "sharding function" ("routing table") for unambiguously deciding whether a given change is for max-next or wingo-next?

I think if the committer wants a particular path they can choose to @-tag one of us. Otherwise ideally we get SnabbBot to @-tag one of us at random. Until we have that kind of bot integration, I think the first person that comments on the ticket and says "I'll take this one" gets it. WDYT @eugeneia ? I'm happy to share any work that you would like to share.

The Linux kernel has the MAINTAINERS file that defines the "first hop" for new incoming changes. We would seem to need some equivalent. I would like contributors to always know what is their next-hop

I guess we add this to our new contributing.md that we were discussing in #729 will have this. I can add that in a separate PR, if that works for you.

I would prefer not to pull the same changes from both max-next and wingo-next.

I was going to initially suggest that the first few wingo-next merges go through max-next, to improve review quality; wdyt? I'm happy either way.

@lukego
Copy link
Member

lukego commented Feb 2, 2016

I was going to initially suggest that the first few wingo-next merges go through max-next

If this suits Max then it suits me very well too.

Then we would have the simple flow:

master <- next <- max <- wingo

and you two could decide on a good criteria for choosing the "first hop" for incoming PRs that makes it clear to everybody.

I think if the committer wants a particular path they can choose to @-tag one of us. Otherwise ideally we get SnabbBot to @-tag one of us at random. Until we have that kind of bot integration, I think the first person that comments on the ticket and says "I'll take this one" gets it.

Can we develop these workflows locally within branches?

I still see us building a network based on the internet here. Each branch is a router, each agreement to pull changes between branches is a patch cable, and each commit being propagated is a packet. The system scales by treating the routers/branches are predictable black boxes that take packets/commits one step closer to their destination. The processing decisions made by the routers/branches are made locally whenever possible.

I am very comfortable with each branch inventing its own maintenance style, for example one branch being operated by a cooperating group who are assisted by a bot. This is like building a new router that the rest of the world can see as a black box with observable throughput/latency/drops/support/etc. If it works really well then lots of traffic can be delegated to it and other routers could borrow its design.

I am more cautious about inventing new protocols for coordinating branches. Then the analogy is to more complex mechanisms like OSPF, BGP, VRRP, etc that multiple routers need to support and interoperate with. I feel like it is too early to be dealing with this kind of complexity in the small network that we have for the immediate future (less than 10 nodes compared with thousands for Linux or millions for the internet).

So I would request that we focus on innovative workflows within branches but make the interactions between branches as boring as possible. Reasonable?

I also note that there will be more kinds of downstream branches in the future and a solution designed for handling individual features PR'd to Github won't cover everything. For example program-specific branches for NFV, lwAFTR, ALX, etc could send large PRs to push their latest releases onto the main branch and it may require a different skill set to process these. (I will resist the temptation to make an analogy to routers in this instance as a courtesy to fatigued readers :-))

@lukego
Copy link
Member

lukego commented Feb 2, 2016

Here are some interesting links from Linux subsystem maintainers:

The picture this paints for me is individuals or groups taking responsibility for maintaining some aspect of the software. Their inputs are simple (e.g. all changes related to GPIO drivers) and their outputs are simple (e.g. publish three branches feeding changes to the testing/unstable/stable upstream branches). The part in the middle is their own domain where they apply thought and creativity.

@lukego
Copy link
Member

lukego commented Feb 2, 2016

... having said all of that :-)

I suppose that the rate of PRs is fairly modest for now and so a "routing function" that requires some social interaction may still be efficient enough. Could also be that one cooperatively-maintained branch will turn out to have really good performance and be able to take pretty much all the individual changes that are sent to Github. I may be mis-predicting what will be important for scaling up in the future.

@wingo
Copy link
Contributor Author

wingo commented Feb 3, 2016

Then we would have the simple flow:
master <- next <- max <- wingo

In the beginning, sure. I expect after the first handfull of merges I would want to go directly to next.

I am more cautious about inventing new protocols for coordinating branches. Then the analogy is to more complex mechanisms like OSPF, BGP, VRRP, etc that multiple routers need to support and interoperate with. I feel like it is too early to be dealing with this kind of complexity in the small network that we have for the immediate future (less than 10 nodes compared with thousands for Linux or millions for the internet).

So I would request that we focus on innovative workflows within branches but make the interactions between branches as boring as possible. Reasonable?

I lost you here :) I think based on your following comments you are suggesting that @-tagging a reviewer is too complicated. However this goes against what you were saying in #725 (comment) and which I agreed with. For all of the reasons that I mentioned in that PR, I do not think partitioning the snabb tree into owners is a good idea at this time.

@eugeneia
Copy link
Member

eugeneia commented Feb 3, 2016

Then we would have the simple flow:
master <- next <- max <- wingo

In the beginning, sure. I expect after the first handfull of merges I would want to go directly to next.

I am find with this too.

As for “routing”, I think its too early to have a good answer. There may be some obvious ownership (Snabb NFV, lwaftr) but I hope that we will be able to self-assign on a “whatever seems reasonable” basis.

I am not convinced by the “random assignment” bot. That is not to say I stand against it, but just to say I would not want to be randomly assigned. Indulging in a PR is something I want to keep self-determined for myself at least (with the exception that anyone is of course welcome to mention me any time, but when I am mentioned I assume there is some motive). I expect the random assignment to produce more noise than do good.

@wingo
Copy link
Contributor Author

wingo commented Feb 3, 2016

A couple final questions. One, I see there is a SnabbCo/max-next. I could manage a SnabbCo/wingo-next but that sounds unnecessary. If it's just in wingo/wingo-next then maybe the second "next" is not needed. Is wingo/next a fine name? If so I can change the branch name then.

Two, what do I do with my commits? I propose that I add them to wingo-next only via PRs. I can get anyone to review them. In particular I want to lean on @kbara and @dpino here. If I feel that I need feedback from @eugeneia or @lukego I will tag you and/or make a PR against max-next.

@eugeneia
Copy link
Member

eugeneia commented Feb 3, 2016

A couple final questions. One, I see there is a SnabbCo/max-next. I could manage a SnabbCo/wingo-next but that sounds unnecessary. If it's just in wingo/wingo-next then maybe the second "next" is not needed. Is wingo/next a fine name? If so I can change the branch name then.

The branches are mirrored on SnabbCo/snabbswitch and this is why they need unique names in that namespace, next is taken.

Two, what do I do with my commits? I propose that I add them to wingo-next only via PRs. I can get anyone to review them. In particular I want to lean on @kbara and @dpino here. If I feel that I need feedback from @eugeneia or @lukego I will tag you and/or make a PR against max-next.

I recommend to open PR's against master as that is what the CI checks and as such the GitHub “merge conflict” message will potentially be misleading, but its not a big issue. Maybe the CI should merge with the target branch instead of always using master?

lukego added a commit to lukego/snabb that referenced this pull request Feb 3, 2016
@lukego lukego merged commit 586f0b7 into snabbco:next Feb 3, 2016
@lukego
Copy link
Member

lukego commented Feb 3, 2016

Sorry to keep you waiting. Your wingo-next branch is live now. You can see it mirrored on the SnabbCo repo so that everybody should pick it up by default. Note: the mirroring script is not picking up rebases (doesn't force-push) so let me know if it gets stuck and intervention is needed.

@lukego
Copy link
Member

lukego commented Feb 3, 2016

You should also have access to Issues e.g. to set yourself as Assignee or apply tags.

@lukego
Copy link
Member

lukego commented Feb 3, 2016

To be clear: you can just push to your own branch and send PRs from there. The mirror of wingo-next on SnabbCo is just a convenient read-only mirror of your branch that is updated each ~1minute.

I will shift my attention now from writing long blah blah blah diatribes to working on high throughput low latency merges with an acceptable rate of mistakes :)

@wingo
Copy link
Contributor Author

wingo commented Feb 4, 2016

Thanks for the merge, let's give things a go! Wheee!

However! On one point I think I didn't express myself right, when I said:

Two, what do I do with my commits? I propose that I add them to wingo-next only via PRs. I can get anyone to review them. In particular I want to lean on @kbara and @dpino here. If I feel that I need feedback from @eugeneia or @lukego I will tag you and/or make a PR against max-next.

My question was about code that I write, not code that I merge in. I assume that for code that I write I need some kind of review process. What is that process? Above is my proposal.

@lukego
Copy link
Member

lukego commented Feb 4, 2016

As for “routing”, I think its too early to have a good answer. There may be some obvious ownership (Snabb NFV, lwaftr) but I hope that we will be able to self-assign on a “whatever seems reasonable” basis.

I wrote a braindump on what I think the downstream people submitting changes are expecting: #737 (comment)

Does that seem reasonable? If so then every currently open PR should be assigned to either @wingo or @eugeneia as the upstream who will engage with the submitter? (How will we make that happen?)

@wingo
Copy link
Contributor Author

wingo commented Feb 4, 2016

Since both I and @eugeneia can use the assignee field, we can use that. There are only 19 PRs so we can probably just go through and assign ourselves. It makes some weird incentives though -- some part of me wants no open PRs assigned to me, but obviously it's the submitter's job to make a nice PR. I guess we should feel free to close PRs that are going nowhere, knowing they can be re-opened.

@lukego
Copy link
Member

lukego commented Feb 4, 2016

It will be interesting to see a workflow develop between individual contributors and the upstream maintainers :-). Tags on issues are another tool in the maintainer-toolbox that might come in handy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants