Maslow Home Maslow Community Garden

Let's review the Robot-auto-merge system

groundcontrol
communitygarden
firmware
robot

#1

Here are three things we should consider about how our very useful Community Garden Robot performs their duties.

A voting period of 48 hours is reasonable for most of the Community Garden projects as thorough testing isn’t needed. The Firmware, GroundControl and Electronics projects are different, in my opinion - they’re core projects. Changes to those need to be tested on multiple platforms and a lot of end user problems can arise if the PR is included in a release without testing. I would suggest that the voting period be extended to two weeks for these core projects, to give time for testing and community discussion.

As @johnboiles found recently,

It’s probably harder to do, but closing and re-opening the PR should restart the timer.

Finally, “I’m just a robot, but I love to see people contributing so I’m going vote thumbs up!” I’m not sure whether the robot votes in order to set up something needed when the votes get counted, but the outcome is that PRs are favored to merge. In most cases that is a good idea, but I don’t think that is appropriate for these core projects.

What do you think about these points? What other things should we consider?


#2

Yes! I think this is a great point and a very important discussion. The robot has been in use long enough now that we should review how it works. Thank you for bringing this up.

I agree. 48 hours is too fast especially now that we have a longer release cycle. Two weeks seems long to me but let’s give it a go. How about we add a “voting time” option to the ROBOT.md file and that way we can tweak the voting time with a simple pull request for any project.

I agree. On a longer timeline this might be less of an issue.

I like that pull requests are favored to merge. The reason that the robot does a :+1: by default is that I wanted it so that if nobody checked anything pull requests would be merged automatically. Ties are not merged so even one :-1: can stop the pull request. I do think that a slight bias towards merging is a good thing. I’ve made pull requests to other open source projects and had them just sit open forever because nobody was paying attention so I’d like to see the robot force the community to either vote or let anyone who wants to contribute to do so.


#3

The author of the PR should vote - there’s the desired bias toward merging, if the community ignores it, it would merge. Having the robot add a vote means that there is one vote that does not represent testing or evaluation of the content, which might not matter for simpler projects, but I feel isn’t appropriate for these core ones.


#4

I’m worried that sometimes the author of the project won’t know how to vote. It is a little counter intuitive. Having the robot cast the first vote it makes it clear how voting should be done. I see what you are saying about biasing too much towards merging for the core projects. I also don’t want to be in a position where an ignored pull request won’t merge


#5

I guess it seems reasonable to me to expect a contributor to understand and use the voting system. The robot can register a ‘demonstration vote’ to demonstrate the process, but should subtract its vote back out.


#6

Subtracting it’s vote back does seem reasonable. It could cast a vote and then subtract one vote from the total for determining a merge


#7

The email sent to a PR author by the MaslowCommunityGardenRobot (needs a name - how about a contest?) should urge them to vote.


#8

Unfortunately that email is sent by GitHub automatically so I don’t think we can mess with it too much. It is nice that we don’t need to worry about sending emails though.


#9

How about a compromise on the voting issue. If you open a pull request to change the way the votes are tallied (around line 87 in this file) I will gladly give it my vote. Changes to the Community Garden Robot files are automatically applied right away so any change to that file will effect how the robot works as soon as the PR is merged


#10

PR created, closed, corrected, re-created :crazy_face:, waiting for the polls to open - gonna vote!


#11

If the author can’t figure out how to vote for it, how can we be sure that other people will know how to vote against it? (as I didn’t until yesterday)


#12

or can it vote both +1 and -1?


#13

add to the comment that to vote you click on the smily face and select thumbs up or down on that comment


#14

This seems very long to me. In my case I had a stack of commits I wanted to propose that I pulled from @madgizzle’s fork several of which depended on each other. But for ease of review I wanted them to be in different PRs per feature. The easiest route is to propose them one after another since then they’d merge in cleanly if they were all accepted. If I wanted to propose them all at the same time, I’d have to rework the commit history so that they were each based on master, and then handle merge conflicts (undo the commit history rework) when it came time to merge.


#15

My suggestion is that we either allow proposing multiple features per PR. OR we shorten the merge time window to 1 week or less.

Git wrangling out discrete change sets becomes a big task in itself.


#16

I think this is a good idea since then it also becomes obvious how to downvote. And we should encourage the author to vote themselves in the messaging from the bot. It wasn’t clear to me if it was acceptable etiquette to vote for yourself — it felt like ‘liking’ my own post


#17

I’m thinking in specific of a series of commits that renamed many of the functions and changed the operation of some of them. Among them was a fix for the G2/G3 gcode function that made it work correctly with the Z axis. There was no way to accept that change without accepting all the others. That doesn’t seem like a good thing.

Every part of every PR should be checked and tested by users other than the author, on all platforms if that’s appropriate. Expecting that to happen in two days isn’t reasonable. I doubt that even one week is, especially if changes come in groups. A good discussion on the forum can take a couple weeks (months?) to reach consensus. Change is exciting, but people who use a release want it to work properly, and that requires that the changes be tested.


#18

I remember the times one bug was fixed and 2 new ones planted every week :rofl: (exaggerated)

To go from developer to ‘stable’ release, testing on min. 3 platforms/persons was not given and from my dementia memory the numbers of programmers and testers was higher than now.
How many testers we wave some how correlates to speed of merging for me.


#19

I think it’s good to requier the author to vote for it to get it merged. This
allows for the posting of ‘RFC’ type PR’s that are not ready to be merged as-is.

If the author thinks it’s worth merging, and votes to do so, that provides the
bias towards merging that Bar wants.

David Lang


#20

I think we need to have a group of people who can merge, so that the robot
auto-merge is only a fallback to be used for less significant PRs that would
otherwise stagnate.

David Lang