# Let's review the Robot-auto-merge system

**URL:** <https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329>\
**Category:** Site Feedback\
**Tags:** groundcontrol, communitygarden, firmware, robot\
**Created:** [October 18, 2018, 6:59pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329 "2018-10-18T18:59:23Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![blurfl](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/blurfl/32/9_2.png) [@blurfl](https://forums.maslowcnc.com/u/blurfl)\
**Post date:** [October 18, 2018, 6:59pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/1 "2018-10-18T18:59:23Z")

</div>

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,

> [@Top Beam Tilt Correction Pull-Request](https://forums.maslowcnc.com/t/top-beam-tilt-correction-pull-request/7307/32):
>
> vote is counted 48 hrs from when the PR is originally opened, not when it was re-opened.

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?

---

<div class="post-metadata">

**Author:** ![bar](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/bar/32/10_2.png) [@bar](https://forums.maslowcnc.com/u/bar)\
**Post date:** [October 19, 2018, 5:38pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/2 "2018-10-19T17:38:32Z")

</div>

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.

> [@blurfl](#):
>
> 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.

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.

> [@blurfl](#):
>
> It’s probably harder to do, but closing and re-opening the PR should restart the timer.

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

> [@blurfl](#):
>
> 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.

I like that pull requests are favored to merge. The reason that the robot does a 👍 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 👎 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.

---

<div class="post-metadata">

**Author:** ![blurfl](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/blurfl/32/9_2.png) [@blurfl](https://forums.maslowcnc.com/u/blurfl)\
**Post date:** [October 19, 2018, 6:04pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/3 "2018-10-19T18:04:59Z")

</div>

> [@bar](#):
>
> I’ve made pull requests to other open source projects and had them just sit open forever because nobody was paying attention

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.

---

<div class="post-metadata">

**Author:** ![bar](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/bar/32/10_2.png) [@bar](https://forums.maslowcnc.com/u/bar)\
**Post date:** [October 19, 2018, 6:08pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/4 "2018-10-19T18:08:32Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![blurfl](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/blurfl/32/9_2.png) [@blurfl](https://forums.maslowcnc.com/u/blurfl)\
**Post date:** [October 19, 2018, 6:14pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/5 "2018-10-19T18:14:42Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![bar](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/bar/32/10_2.png) [@bar](https://forums.maslowcnc.com/u/bar)\
**Post date:** [October 19, 2018, 6:16pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/6 "2018-10-19T18:16:27Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![blurfl](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/blurfl/32/9_2.png) [@blurfl](https://forums.maslowcnc.com/u/blurfl)\
**Post date:** [October 19, 2018, 6:21pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/7 "2018-10-19T18:21:01Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![bar](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/bar/32/10_2.png) [@bar](https://forums.maslowcnc.com/u/bar)\
**Post date:** [October 19, 2018, 6:24pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/8 "2018-10-19T18:24:11Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![bar](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/bar/32/10_2.png) [@bar](https://forums.maslowcnc.com/u/bar)\
**Post date:** [October 19, 2018, 6:26pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/9 "2018-10-19T18:26:08Z")

</div>

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](https://github.com/MaslowCommunityGarden/Community-Garden-Robot/blob/master/Scripts/robot.py)) 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

---

<div class="post-metadata">

**Author:** ![blurfl](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/blurfl/32/9_2.png) [@blurfl](https://forums.maslowcnc.com/u/blurfl)\
**Post date:** [October 19, 2018, 7:04pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/10 "2018-10-19T19:04:10Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![dlang](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/dlang/32/859_2.png) [@dlang](https://forums.maslowcnc.com/u/dlang)\
**Post date:** [October 19, 2018, 7:20pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/11 "2018-10-19T19:20:40Z")

</div>

> [@bar](#):
>
> I’m worried that sometimes the author of the project won’t know how to vote. It is a little counter intuitive.

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)

---

<div class="post-metadata">

**Author:** ![dlang](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/dlang/32/859_2.png) [@dlang](https://forums.maslowcnc.com/u/dlang)\
**Post date:** [October 19, 2018, 7:21pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/12 "2018-10-19T19:21:24Z")

</div>

> [@bar](#):
>
> 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

or can it vote both +1 and -1?

---

<div class="post-metadata">

**Author:** ![dlang](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/dlang/32/859_2.png) [@dlang](https://forums.maslowcnc.com/u/dlang)\
**Post date:** [October 19, 2018, 7:23pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/13 "2018-10-19T19:23:39Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![johnboiles](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/johnboiles/32/7844_2.png) [@johnboiles](https://forums.maslowcnc.com/u/johnboiles)\
**Post date:** [October 21, 2018, 4:22pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/14 "2018-10-21T16:22:40Z")

</div>

> [@bar](#):
>
> Two weeks seems long to me but let’s give it a go.

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.

---

<div class="post-metadata">

**Author:** ![johnboiles](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/johnboiles/32/7844_2.png) [@johnboiles](https://forums.maslowcnc.com/u/johnboiles)\
**Post date:** [October 21, 2018, 4:25pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/15 "2018-10-21T16:25:15Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![johnboiles](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/johnboiles/32/7844_2.png) [@johnboiles](https://forums.maslowcnc.com/u/johnboiles)\
**Post date:** [October 21, 2018, 5:07pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/16 "2018-10-21T17:07:48Z")

</div>

> [@dlang](#):
>
> or can it vote both +1 and -1?

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

---

<div class="post-metadata">

**Author:** ![blurfl](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/blurfl/32/9_2.png) [@blurfl](https://forums.maslowcnc.com/u/blurfl)\
**Post date:** [October 21, 2018, 5:41pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/17 "2018-10-21T17:41:15Z")

</div>

> [@johnboiles](#):
>
> a stack of commits… several of which depended on each other.

> [@johnboiles](#):
>
> allow proposing multiple features per PR. OR we shorten the merge time window to 1 week or less.

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.

---

<div class="post-metadata">

**Author:** ![Gero](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/gero/32/17487_2.png) [@Gero](https://forums.maslowcnc.com/u/Gero)\
**Post date:** [October 21, 2018, 6:06pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/18 "2018-10-21T18:06:24Z")

</div>

> [@blurfl](#):
>
> people who use a release want it to work properly, and that requires that the changes be tested

I remember the times one bug was fixed and 2 new ones planted every week 🤣 (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.

---

<div class="post-metadata">

**Author:** ![dlang](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/dlang/32/859_2.png) [@dlang](https://forums.maslowcnc.com/u/dlang)\
**Post date:** [October 21, 2018, 10:49pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/19 "2018-10-21T22:49:50Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![dlang](https://yyz1.discourse-cdn.com/flex031/user_avatar/forums.maslowcnc.com/dlang/32/859_2.png) [@dlang](https://forums.maslowcnc.com/u/dlang)\
**Post date:** [October 21, 2018, 10:54pm UTC](https://forums.maslowcnc.com/t/lets-review-the-robot-auto-merge-system/7329/20 "2018-10-21T22:54:10Z")

</div>

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
