# Robot feature request for merges

**URL:** <https://discourse.vtk.org/t/robot-feature-request-for-merges/10656>\
**Category:** Development\
**Tags:** proposal\
**Created:** [February 10, 2023, 6:28pm UTC](https://discourse.vtk.org/t/robot-feature-request-for-merges/10656 "2023-02-10T18:28:48Z")\
**Posts on this page:** 5\
**Page:** 1

<div class="post-metadata">

**Author:** ![dgobbi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/dgobbi/32/18_2.png) [@dgobbi](https://discourse.vtk.org/u/dgobbi)\
**Post date:** [February 10, 2023, 6:28pm UTC](https://discourse.vtk.org/t/robot-feature-request-for-merges/10656/1 "2023-02-10T18:28:48Z")

</div>

This is for devs and contributors.

As we all know, the MR robot is not only noisy, but also doesn’t always give the best advice. And usually we cut it some slack because it’s just a robot and is therefore blameless for its actions.

I have three suggestions that might make the robot more helpful, and would be very thankful to whoever implements them.

1. The “`Do: reformat`” could add a newline character to files that don’t have one, e.g. “`sed -i -e '$a\' <filename>`” before or after “`clang-format`”.
2. It could also be helpful if “`Do: reformat`” provided some cut-and-pastable commands for fetching the reformat, like the way gitlab gives commands for checking out a topic branch. New contributors to VTK are, unfortunately, likely to do “`git pull`” which results in a mess of conflicts and frustration.
3. Merge commits in MRs can potentially bring in unexpected code, if not caught before the MR itself is merged. At the very least, merge commits should cause a robot warning, and at best, MR’s with merge requests should be rejected (with certain people able to override for the sake of the third-party libraries).

---

<div class="post-metadata">

**Author:** ![ben.boeckel](https://discourse.vtk.org/letter_avatar_proxy/v4/letter/b/ea5d25/32.png) [@ben.boeckel](https://discourse.vtk.org/u/ben.boeckel)\
**Post date:** [February 11, 2023, 12:46am UTC](https://discourse.vtk.org/t/robot-feature-request-for-merges/10656/2 "2023-02-11T00:46:51Z")

</div>

> [@dgobbi](#):
>
> The “`Do: reformat`” could add a newline character to files that don’t have one, e.g. “`sed -i -e '$a\' <filename>`” before or after “`clang-format`”.

We actually require this for all files. An “eol” formatter [in this repo](https://gitlab.kitware.com/utils/source-formatters/) would allow us to enable such formatting for all files (not just `clang-format`-seen files).

> [@dgobbi](#):
>
> It could also be helpful if “`Do: reformat`” provided some cut-and-pastable commands for fetching the reformat, like the way gitlab gives commands for checking out a topic branch. New contributors to VTK are, unfortunately, likely to do “`git pull`” which results in a mess of conflicts and frustration.

We can do this for `SetupForDevelopment.sh`, but not in general (as we don’t know what the remote is named in general). I suppose we could just dump the URL and use `FETCH_HEAD` though.

> [@dgobbi](#):
>
> Merge commits in MRs can potentially bring in unexpected code, if not caught before the MR itself is merged. At the very least, merge commits should cause a robot warning, and at best, MR’s with merge requests should be rejected (with certain people able to override for the sake of the third-party libraries).

There is a check that bans merge commits. However, they are necessary for backport MRs with conflicts and third-party updates. What I can look at is finding “twisted” merges where a first-parent history commit shows up as a non-first-parent. There is [an issue](https://gitlab.kitware.com/utils/rust-git-checks/-/issues/12) to find and ban these.

---

<div class="post-metadata">

**Author:** ![mwestphal](https://discourse.vtk.org/user_avatar/discourse.vtk.org/mwestphal/32/19_2.png) [@mwestphal](https://discourse.vtk.org/u/mwestphal)\
**Post date:** [February 13, 2023, 10:33am UTC](https://discourse.vtk.org/t/robot-feature-request-for-merges/10656/3 "2023-02-13T10:33:17Z")

</div>

I think we should improve the robot so that minimal human intervention is needed and there is less frustration for contributors.

Here is what I envision.

- MR Created

```auto
Thanks for creating a MR for project, it looks like you did not run SetupForDevelopement.sh before creating this MR. please read develop.md and then run the script and push again to your branch.

- commands to do that

```

- Contributor does that and force push

```auto
It looks like you created a MR from your master branch. Please push to a new branch and change the targets of this MR ... (more info on how to do that)

```

- Contributor does it and robot run again

```auto
Formatting check ... please run Do: reformat and then reset your local repository (with example commands)

```

- Contributor does that

```auto
Test check: Please run the test by using Do: test or manually running the tests in the pipelines tab. You can see the results in the cdash link. More info about reading test results on develop.md

```

- Contributor does that, CI is green

```auto
Please request a review from a developer, I will fetch one for you: @mwestphal PTAL.

```

I know, maybe this is too much but at this current state of the robot, I always need to explain to contributors how to work. This should be automated as much as possible and develop.md should have at least a TLDR section to explain to contributors what they need to do.

---

<div class="post-metadata">

**Author:** ![mwestphal](https://discourse.vtk.org/user_avatar/discourse.vtk.org/mwestphal/32/19_2.png) [@mwestphal](https://discourse.vtk.org/u/mwestphal)\
**Post date:** [February 13, 2023, 10:33am UTC](https://discourse.vtk.org/t/robot-feature-request-for-merges/10656/4 "2023-02-13T10:33:35Z")

</div>

@Francois_Mazen

---

<div class="post-metadata">

**Author:** ![ben.boeckel](https://discourse.vtk.org/letter_avatar_proxy/v4/letter/b/ea5d25/32.png) [@ben.boeckel](https://discourse.vtk.org/u/ben.boeckel)\
**Post date:** [April 24, 2023, 2:53am UTC](https://discourse.vtk.org/t/robot-feature-request-for-merges/10656/5 "2023-04-24T02:53:59Z")

</div>

> [@mwestphal](#):
>
> This should be automated as much as possible and develop.md should have at least a TLDR section to explain to contributors what they need to do.

Let’s first get the docs written up. Once we have words there, we can have the robot learn how to do that. Some of this is covered by [this issue](https://gitlab.kitware.com/utils/ghostflow-director/-/issues/19) (though adding details from here to there would be good to not lose them).

Other bits just seem…chatty. Solutions also exist:

- branch named `master`: use `Topic-rename` in the description to change the name the robot uses for merging purposes
- reformatting…this is a check and the part that comments only gets back a “fail”; there’s not really structured information there to know why anything in particular is failing (it’s why the instructions are part of the check configuration)
- the robot doesn’t currently listen to pipeline events (though I think it might receive them, they are ignored), so knowing when “CI is green” is not possible without polling (which we try not to do).
