More Code Maintainers Needed

CyanWorlds.com Engine Project Management
Post Reply
Christian Walther
Member
Posts: 317
Joined: Sat Dec 13, 2008 10:54 am

More Code Maintainers Needed

Post by Christian Walther »

There has been talk about compiling a list of “open positions” regarding the open-source Uru development effort at OpenUru.org. I was planning to wait for that, but since it hasn’t materialized yet, I’m going ahead with something that has been on my mind for a while:

A position where OpenUru.org is sorely lacking is code maintainers: People who have push access to the repositories and can accept pull requests and patches. Not necessarily active code contributors themselves, and not necessarily experts enough to review and test all contributions by themselves, but able to organize reviews and act on reviews done by others.

Right now, OU has essentially one person in that position. That creates a single-point-of-failure situation. Rarified is doing a fine job at it, but he obviously has limited time for it besides real life and his other duties as shard admin, foundry admin, server developer etc. As a result, things progress very slowly. Pull requests are being left without activity for two months.* This is not a very inviting atmosphere for contributors. Adding more manpower could make a difference here.

Would my services in this capacity be welcome? I have limited spare time as well, so I alone can’t make much of a difference, but it would be a start.

Things don’t even need to go much faster, as the limiting factor is still Cyan at the end of the chain, who is slow (no judgement implied in that, just a statement of fact). But let’s not be even slower than Cyan. We shouldn’t be the bottleneck.

* In this particular case, other reasons come into play, as D'Lanor notes, but that is a different discussion that I plan to start shortly. Edit: here.
Last edited by Christian Walther on Wed Apr 04, 2012 8:46 pm, edited 2 times in total.
User avatar
Mac_Fife
Member
Posts: 1239
Joined: Fri Dec 19, 2008 12:38 am
Location: Scotland
Contact:

Re: More Code Maintainers Needed

Post by Mac_Fife »

You're right, Christian, we've talked about posting these open positions for some time. Work did start on fleshing those out the descriptions for those, but then (ironically) changes of real-life positions impacted on those who were undertaking that task.

We like people who volunteer for things :D

We also recognize that dependency on rarified alone for the tasks you mention is a dangerous position to be in. The role you're describing is the one we currently have on the draft list as "Backup Gatekeeper". It'll probably really be down to rarified in this case for a final say in who he's happy to have in charge of the repos on his server, but I think you've already demonstrated your knowledge in that respect.

I think we all understand the time limitations of volunteers for anything: We're all in the same position one way or another.
Mac_Fife
OpenUru.org wiki wrangler
User avatar
JWPlatt
Member
Posts: 1137
Joined: Sun Dec 07, 2008 7:32 pm
Location: Everywhere, all at once

Re: More Code Maintainers Needed

Post by JWPlatt »

We like self-starters, Christian. No need to wait for some official thing to take the initiative. I've written about that several times in terms of other contributors. I'm looking forward to discussion on this, but not a long one. ;) Rarified should respond later tonight.
Perfect speed is being there.
User avatar
rarified
Member
Posts: 1061
Joined: Tue Dec 16, 2008 10:48 pm
Location: Colorado, US

Re: More Code Maintainers Needed

Post by rarified »

Hi Christian,

You accurately describe the situation just as it is painfully obvious to me. I'd welcome your help in the gatekeeping tasks.

Workflow: I have settled on a workflow described by these steps:
  1. A developer issues a pull request on one of the *-ou repositories. This is the trigger event I use to extract the changeset from the developer's fork repository, and create a changeset for the *-ou-minkata repositories, as well as upload the patch to Fisheye/Crucible for a review. Your point about an increased liklihood of error is taken, but I don't think it is as bad as you described, more later on this.
  2. When one or more changes are made to the CWE-ou-minkata workspace, an automated build is started by Jenkins.
  3. Promote the build. If the Jenkins build is successful, it becomes a candidate to integrate into Minkata. This involves retrieving the client binary from Jenkins, replacing it in the Minkata server tree, rebuilding the manifests on the server, and confirming that running the prior game client now autoupdates itself to the updated client successfully.
  4. Complete the review. While the code is available on Minkata (unless it fails dramatically), conduct the source review or receive review comments in Crucible. After comments are complete on Crucible, as well as feedback from the users on Minkata that the feature operates properly, close the Crucible review.
  5. Promote the feature to the *-ou repositoriesWhen the Crucible review is closed, go back to BitBucket (if that is the origin of the pull request) and approve the pull request.
This is obviously the happy path through the workflow. Let's look at a few of the exceptions that can happen.
  • The source changes do not match the *-ou repository and the patch fails This has happened in a few cases, sometimes by whitespace changes (I have a custom patch executable that overlooks line ending differences and can accept mixed-mode patches) In this case I'll examine the source code by hand, usually the change can be implemented as either a slight tweak to the patch file, or in more difficult cases, by editing the reference source from the *-ou repositories by hand to match the intent of the patch. In either case, the changes get committed to the *-ou-minkata repository in one or more commits. If the patch extracted from the origin repository (fork) cannot be used to start the Crucible review, a new patch is extracted from the *-ou-minkata repository commit(s) for the review.
  • The source changes match the corresponding *-ou repository, but they do not build on Jenkins with the VS2003.net toolset. This is addressed similarly to the previous case; except it usually is accomplished by hand-editing the affected files and committing the changes to the *-ou-minkata repositories. Crucible, if the review is started, is refreshed with a new patch extracted from the *-ou-minkata repository. Repeat until Jenkins indicates success.
  • The pull request is for a repository other than CWE-ou In this case, much is done by hand at the moment. If MOULSCRIPT-ou is the destination, attempt to integrate the changes as in the happy path, but since Jenkins currently does not create the packed and encrypted archive, it must be done by hand. Preferably the "current" plPythonPack program is used to pack, and the "current" plFileSecure utility is used to encrypt the file. One problem is that the current VS project and solution files do not automatically build those programs, so I need to manually clone a repo from CWE-ou-minkata on a Windows host and build them by hand.(A changeset to add those programs to the list of programs in the "all" target in the solution files would be most welcome, and let me advance on automating this with Jenkins.) If the python scripts compile, pack, and encrypt successfully, then again by hand the server is updated with the new python.pak file, the manifests updated, and the server verified by starting the game client.
In this workflow, the *-ou-minkata repositories are essentially "scratch" repositories -- at any time when all the changes have been integrated back to *-ou, the cruft in *-ou-minkata can be obliterated by replacing it with a new clean clone from *-ou. I'm trying to keep unsuccessful changesets that may appear in *-ou-minkata from the acceptance process from propogating back to the reference *-ou repositories. For that reason I don't directly push from *-ou-minkata to *-ou, and I don't have developers push directly to *-ou-minkata (for which they would need to clone the minkata repo, possibly importing changesets that are discarded at some future time, and the possibly pushing those changes back in a later update to the minkata repository).

It would be great to have you help push changes into the *-ou-minkata repositories to get much closer to promoting them to the shard. Your help would let me concentrate more on completing more of the automation steps (MOULSCRIPT, etc), and reduce the time-to-test.

If you see an obvious (or even not so obvious) way to improve the workflow, while still tolerating some of the exceptions I mention, please let's get that down on this thread and talk about it.

_R
One of the OpenUru toolsmiths... a bookbinder.
Christian Walther
Member
Posts: 317
Joined: Sat Dec 13, 2008 10:54 am

Re: More Code Maintainers Needed

Post by Christian Walther »

My reason for waiting for an “official” request was that for such a powerful position, in the meritocracy that is usual in open-source projects, the approach I’m used to is that the current maintainers invite trusted contributors to committer status, not that would-be committers bring themselves into play. In some communities, asking for privileges is a sure way not to get them.

On the proposed workflow:

After these explanations, I see that the suggestion I made in the other thread fails to take into account the “scratch” status of the *-ou-minkata repositories and the desire to only promote a cleaned-up version of history into *-ou. (Side note: Cleaning up history in Mercurial can be awkward. Git is much better at that. Mercurial fans tend to be of the mindset that rewriting history is a bad thing.) (Side note 2: This also explains why the Minkata-specific customizations are done outside of version control using a patch.)

If I understand correctly, this scheme means that *-ou-minkata will always contain the tip of *-ou. That in turn means that step 1 can be done by pulling and merging and doesn’t require juggling patches, which makes me happier. Crucible too could be pointed at *-ou-minkata rather than requiring patches, but we’d have to see if that is an improvement.

It doesn’t address the issue that pull requests are left open for a long time, but I guess that can be mitigated by giving timely feedback about what is happening to the contribution elsewhere.

In short: This looks workable. Let’s try it. We can make tweaks as we go. Let’s spend time getting things done rather than talking.

I don’t think it’s the responsibility of a maintainer to fix patches that don’t apply cleanly. You just return the patch to the sender, saying “Hey, this doesn’t apply. Please make an updated version that applies to CWE-ou e4757ead78b3.” Same thing if it doesn’t build on VS 2003 – “Hey, this doesn’t build. Here’s the Jenkins error output. Please fix and update your pull request.” (If the maintainer can fix the submission themselves, without wasting time that would better be spent on things only a maintainer can do, then all the better, but it’s not their core responsibility.)
rarified wrote:(A changeset to add [plPythonPack and plFileSecure] to the list of programs in the "all" target in the solution files would be most welcome, and let me advance on automating this with Jenkins.)
I can look into that (as one of the few owners of Visual Studio 2003).

How to proceed?

Here’s what I think the next steps could be – C = things I can do, given the necessary privileges, R = things I’m asking rarified to do because they require privileges I don’t expect to be given.

On CWE:
  • R Strip the top 4 revisions of CWE-ou-minkata (268338d95899 onward), which are a mess. This can also happen later.
  • C Import pull request #6: Basic clipboard functionality into CWE-ou-minkata properly.
  • C Clean up the corresponding Crucible review CWE-9 by removing the messed-up patch and tentatively pointing it at FishEye, if that doesn’t work by adding a new patch.
  • C Import CWE-8 into CWE-ou-minkata.
  • C Quickly look over and build-test CWE-10 and CWE-11, if that succeeds import them into CWE-ou-minkata.
On MOULSCRIPT:
  • I’d like to see the top two revisions of MOULSCRIPT-ou-minkata R stripped and C redone with more helpful commit messages than “# User D'Lanor”, but I don’t insist on that. Preferably by merging the actual revisions that were pull-requested, but rebasing is OK with me too if you prefer a linear history.
  • C Import MOULSCRIPT-20 into MOULSCRIPT-ou-minkata.
  • R At some point, add MOULSCRIPT-ou-minkata to FishEye so that it can be used as a Crucible source repository too (if the above test with CWE-9 indicates that that is useful).
Unless I missed anything, that takes all currently open submissions (Bitbucket pull requests and Crucible patch submissions) into the Minkata stage.

If there are detail questions that are more easily discussed in real time, ping me on IRC.
User avatar
JWPlatt
Member
Posts: 1137
Joined: Sun Dec 07, 2008 7:32 pm
Location: Everywhere, all at once

Re: More Code Maintainers Needed

Post by JWPlatt »

Christian Walther wrote:My reason for waiting for an “official” request was that for such a powerful position, in the meritocracy that is usual in open-source projects, the approach I’m used to is that the current maintainers invite trusted contributors to committer status, not that would-be committers bring themselves into play. In some communities, asking for privileges is a sure way not to get them.
Just to respond here that the likelihood, on the y axis, of such rejection by maintainers is probably on a slowly upward curve starting at 0,0 with the number of contributors/committers on the x axis - and we are much, much nearer to zero than the sort of which you speak. That, and I think you've thoroughly proven your merit beyond any doubt, Christian.
Perfect speed is being there.
User avatar
rarified
Member
Posts: 1061
Joined: Tue Dec 16, 2008 10:48 pm
Location: Colorado, US

Re: More Code Maintainers Needed

Post by rarified »

What you (Christian) are suggesting sounds entirely reasonable, I'll look at rolling things back later today.

However, with regards to the unhelpful comments, there really is more there -- they are multi-line commit messages that don't show up with the normal abbreviated log listings. Either use hg log -v or drill down into the changeset links on the Mercurial web page such as here, corresponding to your example. Yes, the editing was poor when extracting the original comments, but all the meat is there.

I've got meetings this morning so I won't be able to do much till later.

_R
One of the OpenUru toolsmiths... a bookbinder.
User avatar
rarified
Member
Posts: 1061
Joined: Tue Dec 16, 2008 10:48 pm
Location: Colorado, US

Re: More Code Maintainers Needed

Post by rarified »

Let's give this a try... I've given you push access to the *-ou-minkata repositories on the Foundry. You'll need to authenticate with your foundry username and password.
Christian Walther wrote:On CWE:
  • R Strip the top 4 revisions of CWE-ou-minkata (268338d95899 onward), which are a mess. This can also happen later.
Done.
On MOULSCRIPT:
  • I’d like to see the top two revisions of MOULSCRIPT-ou-minkata R stripped and C redone with more helpful commit messages than “# User D'Lanor”, but I don’t insist on that. Preferably by merging the actual revisions that were pull-requested, but rebasing is OK with me too if you prefer a linear history.
    ...
  • R At some point, add MOULSCRIPT-ou-minkata to FishEye so that it can be used as a Crucible source repository too (if the above test with CWE-9 indicates that that is useful).
MOULSCRIPT-ou-minkata is stripped, and I'll look to add it as an anchor repository tomorrow.

Jenkins has been reset to use the stripped repositories and is building now.

I saw your pull request for the VS solution file updates for the python tools, thanks! I'll give that a try later in the weekend.

Thanks for the help with this!
_R
One of the OpenUru toolsmiths... a bookbinder.
Christian Walther
Member
Posts: 317
Joined: Sat Dec 13, 2008 10:54 am

Re: More Code Maintainers Needed

Post by Christian Walther »

Thanks! As much as I can from the list above is done.

Now I need permission to edit CWE-9, and while I’m at it, to close CWE-6 (possibly one and the same).
Christian Walther
Member
Posts: 317
Joined: Sat Dec 13, 2008 10:54 am

Re: More Code Maintainers Needed

Post by Christian Walther »

Some experiences from my new role:

From my experience with CWE-9 and CWE-12, I have found that Crucible works much better when the code changes to be reviewed come from commits in a repository (through FishEye) than when they are added as patch files.

In particular, it nicely lets you review contributions consisting of multiple commits – you can see differences from individual commits, or from everything combined, or anything in between. With patch files, that was a pain, because it wouldn’t let you anchor the patches to each other, meaning that it had no idea of the relationship of the patches among each other and therefore couldn’t show you a combined diff, and of the original codebase and therefore couldn’t give you more context in a diff past the first one. It also wouldn’t show you any commit messages (these are contained in the patch files when generated by hg export), which often contain important information for reviewers.

I therefore suggest that we stop feeding patches to Crucible and only use it on commits.

That also means that opening a Crucible review and adding a patch, as tentatively practiced in MOULSCRIPT-20, CWE-8, CWE-10, CWE-11, is out as a way of accepting contributions. (I’m sure Adam’s experiences there won’t make him disagree with that.) Taking contributions as Mercurial commits, e.g. through pull requests on Bitbucket, is the way to go.
Post Reply

Return to “Management”