Collaborator request about triplea HOT 28 CLOSED

👍

I agree. I could also use contributor rights just for labeling, and promise to push all changes via pull requests.

from triplea.

"collaborator" in git terminology is a "owner", but we're establishing an honor contract to push changes via pull request?

Thinking of a potential ticket migration. IMO tickets are good thing for @veqryn to not own, enough on his plate, @ron-murhammer already I think has a lot of effort into the tickets too. IMO Ron and us three as helper minions could be a useful way to migrate tickets. I think also @djensen47 and @gaborbernat have good experience with organizing github issues, makes me interested to hear more about their ideas on how to organize the tickets in github.

But not to distract too much from the conversation, please if would, do clarify (for us github newbs) how you "collaborator" would be achieved using in github terminology (AFAIK we're labelled collaborators already, so I expect it to mean for something more to be done in github to enable this).

from triplea.

You're right, we are all collaborators. I'm looking for the permissions to edit/label issues. Admin maybe?

from triplea.

Yeah, it appears the issue is that in order to label issues you need the same group as being able to push code changes directly (which we are trying to limit currently). Not sure if there is any way around that and how @veqryn would feel about opening it up based on an honor code.

A somewhat hacky alternative is to just create the issue with prefix to what they should be tagged as. Either way we probably need to come to a general consensus on what tags we want to use. Then when I or Veqryn have time we can actually label them based on the prefixes.

In terms of existing SF tickets, we need to do some amount of clean up IMO before moving them over. I actually went through and closed a fairly large number a few weeks back that were either already addressed or useless. It probably still needs a lot of work as I really would prefer to not migrate ~200 bugs/feature requests. I'm guessing about 25-50% of them are actually useful at this point and the rest should just be closed as either fixed or useless.

from triplea.

So there appears to be one other options which is: "Note: There is no team permission that allows access to issues, but blocks read access to repositories. However, you can create a second repository that contains only issues, and store your code repository in a separate, restricted repository."

Not sure if anyone has done that or seen it done successfully. Wish github let you have more fine grain control of permissions.

from triplea.

I think we can go with honor code mostly at the time being, we are grown up responsible people, if someone goes against it it's quite easy to revoke it's permissions and reset the change :)

from triplea.

Not too many drawbacks that I see from this. I do see quite a few benefits to this:

• would lend more creativity and general effort to some of the more process type of stuff
• spreads work of the tickets around. The source forge ticket queue looks to have gotten out of hand
• spreads responsibilities, boosts community engagement

@veqryn , @ron-murhammer , would be nice to see this decided one way or another.

from triplea.

Will think about this weekend.

from triplea.

@veqryn , I'm having trouble seeing this as an open source project. Let's look at the definition:
"Generally, open source refers to a computer program in which the source code is available to the general public for use and/or modification from its original design. Open-source code is meant to be a collaborative effort, where programmers improve upon the source code and share the changes within the community."

You've been requiring all changes to go through you, yet you conveniently decide which changes you will have time to review. This in effect blocks the community from changing the software. Thus, I don't know if I really can view this as an open source project.

In github, there are three key roles. I've talked to both @ron-murhammer and @gaborbernat about this, and they generally agree:
Owner: like the VP of the US, rarely breaks ties and officially does little else. In github owners are also around to be sure nobody can hijack the project and cause permanent damage.
Contributor: change code, test it, submit PR
Admin: Everything else, operations, ticket curation, reviews, regression testing, help with releases, tagging and git management,

Owner's generally do not do much. Admins are the really useful people and you want as many of them as possible to spread the work around.

Another comment, the conversation earlier in this issue about honor code of not self-merging PRs I think was misguided. The git process works well when any one person never acts in more than one role. So if you have admin access, you should not be merging your own PRs anyways (only real exception is perhaps when helping out with releases and doing something trivial and time sensitive like bumping version number).

Finally, given that the people in question have been engaged with TripleA for years, I fail to see how or where you will find other, better, admins.

Last, if PR's are backing up and you do not have time to review them, the answer to that one is they are merged without review rather than waiting for time to be found to review them. In that train of thought, it is also critical we update our process to have more admins and reviewers, I don't think any one person wants to review every single change that happens from here on out.

from triplea.

I object to some of the things you've written above.
Redrum and I agreed a week or two ago that he did not need me to review / approve many of the changes being made.
As he gets a feel for more parts of the engine, then I am 'needed' less and less.

I am not blocking your work intentionally. I am not sure if you have a job or family or not, but I would guess from the volume of your work recently, that you do not have too many outside commitments. That is great, but Redrum and myself do not have the same amount of time as you.

As far as your PR's are concerned, it would probably help if you weren't all over the place with them, or made them all depend on each other.
Lots of your changes are either large in nature, or are changing core parts of the engine, or are surrounded by issues dealing with the direction of the project in general. These are not to be taken lightly, and I gather from your attitude both here and in your comments, that you think Open Source means that so long as there isn't an obvious bug, all PR's should be merged right away.
Sorry, but when you want to change major things, especially around concepts and process, some things have to get debated, and not all of your ideas might make it in (regardless of how long you've been around Triplea).

This is an established project that has a fairly large user base. We are not some startup where it is fair and relatively easy to pivot, throw out a lot of things, cause chaos for the users. We have to maintain a degree of stability for our users, balancing that with appropriate change.

You have a lot of good ideas, but a good number of them need to be filtered, cleanup, and polished by experience from other people like Redrum and myself. There are also some ideas/choices you have that I disagree with, and for those I am trying to comment on all the many issue tickets you've raised, in order to find a satisfactory solution, or push the decision off until a later date so we can focus on more important things.

from triplea.

Open source to me is the definition I pasted. It is not a project fully controlled by one, or just two people. As is, you guys can freely modify the code base, but the community can not. There is an insurmountable bottleneck where all changes go through just one person who can then choose to arbitrarily spend time or not to review. This selective filter of changes by one person does not make this a community project. To boot, I'd argue that you will never be able to review all changes, and so not all changes even have the possibility of being merged. This fact that some changes just will get rejected not based on merit, but because one person does not have time flies in the face of open source principles*, namely that open source is a meritocracy. And so with that respect, PR starts with no obvious bugs, but also asks if the design is good and logical, if the documentation is present and complete, if tests are present and look complete and reasonable, and last if error handling is present, reasonable and complete.

• Open source literature and notes:
• http://opensource.com/open-source-way
• http://opensource.com/principles

from triplea.

My point is, so long as it is the case that not all code will be accepted, not because the code is bad but because one (or two) people do not have time to review it, it indicates a process and project that is not open source in nature.

from triplea.

My point is, so long as it is the case that not all code will be accepted, not because the code is bad but because one (or two) people do not have time to review it, it indicates a process and project that is not open source in nature.

I'd argue that there is not just one model to open source projects. Have you heard of the BDFL? Guido van Rossum is the Benevolent Dictator for Life for the Python programming language. Python is definitely a community project but there is still the BDFL. I'm not saying we should or that we do use BDFL model but it is a model of open source.

The manner in which collaboration occurs is up to each team. However, you, yourself, are free to fork the project make changes, compile, and distribute (as long as your changes are also open source with the GPL license).

from triplea.

So, do you think that every single PR gets merged into code for all open source projects, and right away to-boot?
I don't have merge rights for Firefox, Ubuntu, Python, etc.

We are a project with 4 developers, of which 2 of them have full commit/merge rights, and the other 2 are brand new. I think that is a pretty good for now.

I am not planning on reviewing every PR, and like I said above, Redrum and I agreed that we can commit PR's without each other's approval under a lot of circumstances. And I can see myself not being needed for any merges fairly soon, though I greatly appreciate him reaching out to me for my opinion on changes that affect the direction of the project.

If and when other dev's get up to speed, and I can trust the quality of their work, we will have more admins.

It just took me several hours to catch up on the literally 3 pages of emails generated by you in the last couple days, and I still haven't responded to every update and issue, let alone had any time to actually look at any of the code in the PR's.

Stop insinuating the I am ignoring your PR's on purpose.

from triplea.

We are a project with 4 developers, of which 2 of them have full commit/merge rights, and the other 2 are brand new. I think that is a pretty good for now.

Agreed.

There is nothing stopping me or anybody else from forking and creating an experimental branch (on their own fork) for crazy future ideas. If the crazy idea works out, has good quality, and there is good test coverage, then it can be merged back into main.

If it doesn't work out, then it doesn't harm the main repo.

from triplea.

While you may not have merge rights, someone will come along and review the PR within something resembling a reasonable time period (maybe a week or so). There is no process that ensures some PRs will starve and never be merged. Let's say one person was heading one of those projects, and had a backlog of 3000 PRs, I don't think you would expect any changes to ever make it through unless they were really special and very important.

@djensen47 I think I have over 30 in-flight branches now with what I consider good and useful changes. I'm for the most TDD, and some of those branches are purely to add new tests. Yet, those changes are not being merged back into main, and it is because we do not have enough capacity to reivew. At the current pace of review, I would wonder when you would guess we would clear the current backlog. Also then consider, if I spend another 3 days and submit 10 more PR, when will those get merged in? Can I expect that work to ever be merged in the next year?

Meanwhile, we have gross inefficiency with setting up Travis, we don't have permission on gitter, we have perhaps half our ticket open unnecessarily because there are no admins to close them, or alternatively they are request for things only admins can do.

With respect to 4 developers, no not really. Developers come and go, contributors don't have much responsibility beyond the PR they submit, so all this project has right now are 2 admins who are also the owners. If you had made me, djensen and gabor an admin three weeks ago, we'd have a working CI Travis build (one has already been done successfully via prototype), we'd have half the issues closed, we'd have all our maps out of source forge and inside of sub repos, and we would already have approval of tools like gitter, and tools like reviewable.io wouldn't have had to wait more than a week before simple approval was given.

@djensen47 , so in summary, yes there is something stopping people from creating experimental branches. If you know the work you will do will never see the light of day, why bother? The experimental branches will be made, whither from neglect, and people will stop wasting their time and move on.

from triplea.

@veqryn , didn't mention this in last comment, but I am deeply sorry you have felt I was intentionally saying you were leaving PRs ignored. I did not intend to insinuate that directly or indirectly. I am stating though that one person is too much of bottleneck to keep PR work efficient and the code base moving along.

from triplea.

You've made many of your branches/PR's depend on each other, even when they don't need to.
And then at the root you have PR's with critical or directional changes.
This is why a good number of them are taking a while.

On top of this, we have been trying to get a release out of the door, and it didn't go so well due to some recent changes made. So if we didn't have time to look at your PR's for the last two weeks, this is the probable cause.

Also, I should point out that for major open source projects, they actually have people who are paid quite well to administer those as their full time jobs.
Redrum and I happen to be getting paid nothing (the hundred or so dollars we get in donations, I donate to charity), and have full time jobs of our own.

from triplea.

I am not planning on reviewing every PR, and like I said above, Redrum and I agreed that we can commit PR's without each other's approval under a lot of circumstances. And I can see myself not being needed for any merges fairly soon, though I greatly appreciate him reaching out to me for my opinion on changes that affect the direction of the project.

So this agreement was done without the other half of the team?

Were the changes really that recent? The DMG file being broken was immediate, but the really bad one was the naval retreat bug, which you introduced yourself. So all the stuff about being able to be confident that a dev will have good code, not needing a review, I think is shallow considering that stuff is in place not because we don't trust "new" developers, but because that is the development process for all development.

I'd note as well that an offer has been extended to help with the release, work has been done to automate the whole thing, yet that offer for help has not been taken.

So yeah, you guys are not getting payed anything, and to some extent my impression is that you're insisting on doing all the work, which creates a bottleneck in process as well as other issues, very notably questionable community ownership.

from triplea.

"If and when other dev's get up to speed, and I can trust the quality of their work, we will have more admins."

Since all changes are done via PR, we have other mechanisms to ensure quality of work. I would first look for dedication, people that have played the game for years are ideal candidates, they have vested interests to keep the game functioning well. People that are not good admins, are college students that are looking for any open source project to contribute to so they can do some coding. Other people who are not good admins are probably lobby mods with no coding experience, yet they actually might be okay since they can help with the ticket queue.

Admin is actually not very much about code. I think it's actually better to have your admins be stronger with git than they are with Java. But notably, I've stated admins shouldn't be merging their own PR. PR has a reason, and its reason is not so we can haze the new people and let the frat senior members go with a pass. So if admins are merging in their own code, they aren't executing properly on an admin role.

IMO, if anyone has been playing the game for any significant amount of time, and is willing to dedicate time, and is willing to help out in any way, it's better to prefer them to be an admin. You'll encourage them to continue participating, and you'll get loads of free labor. Anything bad they can do, can be undone.

from triplea.

I've stated several times before in multiple places, that this release would be the last one that uses the current build process. This was part of our commitment to get a stable build to users before we embarked on really large and potentially hazardous changes.
At the time we started the release, gradle was not capable of handling the full release process, and could only create a jar file. Even if it is now capable, I would still not want to use it, because I am not sure of what potentials issues it might generate. It is much better to try out the new gradle release process on an unstable build (ie: a future 1.9.x.x release).

As far as other help for the release goes, I am not sure how much of it we would have wanted to split up. I believe it was a good idea to have redrum take on and do a full release, by himself, in order to get a feel for everything that needs to be done. In the future, we can split this up more.

The naval retreat issue was a bug introduced by redrum a while ago, in code that introduced two bugs. I backed out one bug, but didn't back out the other bug because noone, myself included, noticed it. This is definitely my fault, but I reject your claim that because I am not perfect, I should just give everyone merge/commit access. I've also stated previously that there is more than just code quality that goes into whether we give commit access.

Your time commitment is great, but you can not just railroad people into accepting your PR's because you make so many of them. Like I stated above, certain types of code changes will be delayed more than others, because they deal with critical portions of the code base, or with the direction of the project.

And you must accept that we have lives outside of TripleA, and that our time is limited during a release.

from triplea.

" but I reject your claim that because I am not perfect, I should just give everyone merge/commit access"

This is not my claim. My claim is we need more than one or two people to do code reviews. Preferably 5 really. That way we could have two reviewers potentially do each PR. For example, if you make me an admin, it wouldn't help with the PR queue at all since I submitted most of them. As stated earlier, it's best to act in exactly one role at any time. If I submitted a PR, then effectively I treat that PR as if I were a contributor only.

So yeah, it should be more than code quality, because some admins will never even touch code. They'll do builds, releases, testing. Other admins may solely stay in the ticket queue. If someone offers to help with any of that, as this ticket is an explicit request for - I would take that help.

from triplea.

In my humble view, both parties are right here at some level. I definitely think @veqryn and @ron-murhammer are trying to do as best as possible. Also @DanVanAtta I see why you might think your work is being ignored. However @DanVanAtta keep in mind that usually the project hasn't been tuned for a such large stream of modification. And while they are adapting over this new stream of changes, albeit it may seem slow, they need to make sure at the end of the day a quality products gets published out, otherwise the user rage will just decrease our user base.

On long run we should have a beta release/testing; however that will take some time, so we need to have some patience until then, and make sure our release is as good as possible. Having PR depend on each other sadly is a great risk, as one may not get accepted; you're out in dry. I prefer doing just single independent PRs, per time, to avoid this issue. If this can't be helped delay doing the second PR until the first gets accepted.

Yes there are many tools we can improve the quality of our product and speed up PR accept rate; travis is one of them thanks for it. However, accepting those must be done with the old already tested workflow; even if it's slower to assure build quality. For quick pull request acceptation there needs to be safety nets in place, otherwise everything will fail eventually. For one, at the moment, I wouldn't allow myself to accept PRs; as I may miss to fully understand some parts of this project. So yeah, it sucks, but patience is required.

PS. Recently I've contributed to some other projects too, and generally strong sense of ownership (due to being BDFL for the project), is present, part of the open source world; and ultimately I think help keep the project do not stray off it's initial course, plus stability. For example on the project Git, I think I made already 2 implementation versions, exchanged 25+ mails for a simple progress notification; and still are not there.

from triplea.

I don't think my work is being ignored. A major problem is not enough capacity in the development pipeline. So little capacity I've even suggested that only Ron or Chris can contribute effectively.

Too few reviewers, brittle code, non-trivial manual tests, those are problems that are self-reinforcing. One way out of them is to try and move faster and have more focus on regression testing.

In fact admin right now for me or @gaborbernat would still be of only limited help. With admin I would upload prior releases, do tagging, help with map migration to sub-modules, generate an OAuth token for travis, and help label issues if needed. So the answer to the problem sadly is not simply make me and Gabor admins, I do wish it were just that simple. On the other hand, that is not to say more admins would be of help. If not for very least, helping with all the non-PR stuff, more admins would be of help.

from triplea.

@veqryn Consider how your leadership shifts the decision of someone to simply submit the features they want in a PR, manually tested and then walk away. That person could then go away to happily play games in the lobby, and then perhaps dwindle away with the rest of the user base. There were only 6 people in the lobby tonight at 10:30 PST (generally it is more active than that, the trend I think is clear and there though).

Alternatively a person can be encouraged to help out with the project, to get more of the project work done faster.

I've think I've said my piece as well as I can now. I appreciate the dialog, not everyone can engage in constructive discussion (particularly when there are disagreements) for this long, so thank you for hearing me out.

from triplea.

Well not really sure where to start here...

@veqryn @DanVanAtta I think there are some things above that probably shouldn't have been said or at least phrased better. I think both of you may want to try to put yourself in the other person's shoes to help get the opposing perspective. I guess I'll try to summarize what I think are some useful points out of this as at the end of the day I think we all enjoy TripleA and want to improve it.

I think we can all agree that we don't have as many contributors as we'd like and therefore not a ton of bandwidth for reviewing/merging a large number of PR's quickly. This really isn't going to change quickly and I think we to some extend to to accept it and figure out the best way to maximize the time each one of us has.

@veqryn I think the main point is that we need to do a better job of communication and setting expectations to minimize the frustration of those that are trying to contribute. I think also when discussing PR's or updating/changing them there needs to be appropriate discussion and justification. Submitting changes over top of someone else's changes without any real discussion is probably going to cause tension (example is the scrolling changes). Also, as owners/admins we need to stay above the fray to some extent. I don't think accusing someone of railroading lots of PR's is really going to help anything and there are better ways to discuss concerns.

@DanVanAtta I think the easiest way to say this is that you've essentially overwhelmed us with the sheer number of changes and PR's in a pretty short period of time. Its also not really fair to compare this project to a project with 100s of contributors and dozens of admins either on how fast PR's are reviewed and accepted. In comparison to the previous year there have been a great deal more changes in the last 2 months (give yourself a pat on the back for driving many of those changes). To be honest I wasn't sure we'd ever make it to github as that was the first thing I suggested 1.5 years back when I first started contributing but here we are. But there is a limit to how fast we can change things as we have only 2 admins, a decent size user base, this is an established project, and we only commit a portion of each of our time to it. Most large open source projects have owners/admins that its their job to maintain the project and they are paid to do it which isn't the case here.

The code base is also pretty brittle and complex which means even small changes can have dramatic effects that can be fairly difficult to track down. We aren't just developing basic CRUD web applications here as we are dealing with complex logic and multi-threading. We also have very limited automated test coverage which until that improves things need to be closely reviewed and tested. The naval retreat bug is a very good example of a small change the broke something seemingly unrelated that we don't have any automated test cases for which really demonstrates these points.

As mentioned above, there are many forms of project management for open source projects. To a certain extent, Veqryn has contributed a large amount of time (more than all of us combined) and been really the only core developer for the last few years so if he wants to run it as BDFL then I think he has that option (though I really don't think that's what he wants). And I can at least clarify for myself that I'm not "insisting on doing all the work" as I would much prefer to just develop the AI and comment on direction of TripleA then have anything to do with releases, reviewing so many PR's, etc. I personally don't think accusing people of things or declaring "I don't know if I really can view this as an open source project" is really beneficial. There are more constructive ways to discuss your concerns.

I think maybe we need to set expectations more clearly and hopefully this is a start:

• PR's will be closely reviewed especially when touching core engine code
• Each PR needs to be tested by either including a unit test or explaining how to test it manually (testing will be done by both the contributor and the reviewer as both are responsible for the PR)
• At this point in time, veqryn and myself will be the only ones merging PR's but encourage everyone to contribute, review, and comment on them (we hope to increase this moving forward)
• In general, veqryn and I will also review/merge each other's pull requests except when deemed to be low risk (AI code, changelog, etc) and have either been open for a few days or are urgent

Given the above, here are some things that I suggest you do to assist with and improve the process:

• Minimize having PR's dependent on each other so that they are as independent as possible since this makes them more complicated and blocks all PR's that are dependent on one that is waiting on review. Some case you'll need to have dependencies such as when you add new test libraries and then use these in test cases but the dependencies should be necessary and clear.
• Need to find a way to clearly indicate the priority of PR's when there are a lot of them and which ones are dependent on others. Not sure the best way to do this but even a simple wiki page or issues that describes the current priority when you have say more than 10 PR's open would go a long way.
• Consider focusing on fewer areas of the code. It would be better to pick a few things and make significant improvements then make 100 small refactor changes in lots of different places. The biggest reason for this that its much easier to review. You are free to do as you please but the latter will end up with PR's taking longer to review/merge and most likely more frustration.
• Please make PR's as easy as possible to test and make commits that are easy to review (separate commits for refactoring vs functional changes recommended). Commits should be focused on 1 thing while PR's may include several commits when its all related functionality.
• In general, we want good discussions but also need to focus on being able to get things accomplished as well. To this end, please try to make posts on issues and pull requests more concise so that we can spend more time developing/reviewing and less time reading through tons of posts. It would probably help to think things through a little more since a good number of your posts end up being sort of like a thought stream which takes a while to read through.
• When changing subjective existing behavior, please consider creating forum posts/polls to discuss with users. A good example of this is the scrolling stuff. You and veqryn obviously have different preferences and are only 2 users and we don't really know what the majority of users want. If you don't have data to backup your change and there is disagreement amongst the developers on it then we stick with status quo in general.
• Consider increasing your contributions in areas besides PRs especially when a backlog of your PRs exist. As continuing to add more and more PRs isn't really going to help anyone and just make you feel more frustrated. If you want a list of suggestions, I'd be happy to give some :)

Sorry for the long post. Hopefully this helps clarify some things. My intent is for us to figure out the best way for us all to work together and avoid pointing fingers at each other.

from triplea.

Thank you @ron-murhammer for the well thought out response and well needed advice. I apologize for how I raised the question of community involvement. I also owe you an apology @veqryn and @ron-murhammer for being overly confrontational in general. As you pointed out Ron, we made a good bit of progress recently, I've been pushing you guys hard to try and get things moving. That of course has created tension. Combine that with us being a new team (generally teams fight a little bit at first while people are figuring each other out), and it is recipe for us to struggle and not enjoy this process very much. I don't think that will continue be the case and that we're learning how to work more effectively together.

I've learned a lot recently about open source software ownership models. Particularly the BDFL model. So you guys have the right to run the project in that matter if you so wish. Overall there has been good stewardship of tripleA, I don't think there is any danger of a project fork.

My ultimate frustration is for where I think this game is going, the player base, and the community. It seems like it is dwindling. Without fresh development efforts to improve the core game experience, get it so games can be played faster, improvements to make the game more enjoyable to play - that we will continue to see a diminishing player base.

As you guys know, the game needs a lot of work. Particularly it needs a whole lot of untangling work first before much else can be done. So the frustration is that by seeing you guys place yourselves as bottlenecks, without too much effort to relieve that, that we may potentially never make enough progress to really get anywhere - and that is a frustrating thought.

So the only response I have is to try and encourage as much as possible a collaborative atmosphere. Say thanks when you receive a PR, frame questions about them politely, and don't mind so much if a queue forms - I know you'll work through it eventually (it takes far more time to create a PR than it does to review one, so the queue should drain eventually). So thanks for your patience that you've shown as well.

from triplea.

Any objections if this is closed @djensen47 , @ron-murhammer , or @veqryn ? I think we talked the issue to death and have reached the conclusion admin will not be handed out readily (which has both pros and cons). so it does not seem there are any further pending items to talk about or actions to take regarding this issue.

from triplea.