Skip to content
Snippets Groups Projects

Add Merge Request template

Merged Add Merge Request template
mr_template into master
3 unresolved threads
Merged Cyril Danilevski requested to merge mr_template into master
3 unresolved threads

Hi, I propose to use a MR template within the project in order to make sure that we do not skip information that might be relevant when asking for reviews.

As it's a template, it can easily be modified or removed when not needed (case in point: I'm not using it now). We'd need to have it in master, then activate the option in the repo.

See: https://git.xfel.eu/gitlab/help/user/project/description_templates

It's a template I've used in other projects, or here , which I think exhaustive enough.

@ahmedk @kamile @amunnich @tmichela

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
.gitlab/merge_request_templates/default.md 0 → 100644
1 <!--- Provide a general summary of your changes in the Title above.
2 If it is specific to a detector, indicate which in square brackets.
3 If it is specific to an algorithm, indicate which in square brackets, eg:
4 [AGIPD] [DARK] add plotting -->
5
6 ## Description
7 <!--- Describe your changes in detail
8 Why is this change required? What problem does it solve?
9 If it fixes an open issue, please link to the issue here. -->
10
11 ## How Has This Been Tested?
12 <!--- Please describe in detail how you tested your changes.
13 Include details of your testing environment, tests ran to see how
14 your change affects other areas of the code, etc. -->
15
  • Karim Ahmed
    Karim Ahmed @ahmedk ·
    Owner

    Very nice idea! beside comment LGTM

  • Cyril Danilevski added 1 commit

    added 1 commit

    Compare with previous version

  • Karim Ahmed
  • Cyril Danilevski added 1 commit

    added 1 commit

    Compare with previous version

  • Hugo Santos
    Hugo Santos @santosh ·

    2 cents from my side: I already worked with this kind of checklist when I was still using SVN (08'). It does help, for sure, but...

    1. Make sure to "keep it as simple as possible" - no verbose
    2. For hotfixes, keep it even more straightforward (usually, you do it in a rush)
    3. To be honest, sometimes it is a bit annoying to provide it and it slows down the MR
    4. Nothing replaces a well written and commented code.
    5. Once in a rush, stressed and under pressure, processes and documentation are "throw out through the window" - every process checkpoint must be also thought for stressing times.

    @kamile @kluyvert @tmichela - comments?

    @danilevc I would start it with yourself as a prototype before having it established for the entire team. Therefore, you can be practical measuring the benefits (@ahmedk already gave you the LGTM anyways)

    Edited by Hugo Santos
  • Thomas Kluyver
  • Thomas Kluyver
  • Thomas Kluyver
    Thomas Kluyver @kluyvert ·
    Owner

    I've put in a couple of suggestions trying to make it shorter, as I prefer concision for things like this. But they really are suggestions - feel free to leave them if you don't think they're improvements.

  • Cyril Danilevski added 1 commit

    added 1 commit

    • 0574d8e5 - Apply suggestion to .gitlab/merge_request_templates/default.md

    Compare with previous version

  • Cyril Danilevski added 1 commit

    added 1 commit

    • aebc2a2d - Apply suggestion to .gitlab/merge_request_templates/default.md

    Compare with previous version

  • Cyril Danilevski added 1 commit

    added 1 commit

    • df483141 - Change checklist to not be tasks

    Compare with previous version

  • Thomas Kluyver
    Thomas Kluyver @kluyvert started a thread on the diff
    • .gitlab/merge_request_templates/default.md 0 → 100644
    11 your change affects other areas of the code, etc. -->
    12
    13 ## Relevant Documents (optional)
    14 <!-- Include any relevant screenshot, elogs, reports, if appropriate. -->
    15
    16 ## Types of changes
    17 <!--- What types of changes does your code introduce? Uncomment all lines that apply: -->
    18
    19 <!-- - Bug fix (non-breaking change which fixes an issue) -->
    20 <!-- - New feature (non-breaking change which adds functionality) -->
    21 <!-- - Breaking change (fix or feature that would cause existing functionality to not work as expected) -->
    22
    23 ## Checklist:
    24 <!--- Go over all the following points, and uncomment all lines that apply: -->
    25
    26 <!-- - My code follows the code style of this project. -->
    • Thomas Kluyver
      Thomas Kluyver @kluyvert ·
      Owner

      Do we have a strict coding style? A tool to check it?

      I personally tend to like loose coding styles, with some room to decide how to format code clearly. But if we're going to have this in a checklist, we should be on the same page about what it is.

    • Cyril Danilevski
      Cyril Danilevski @danilevc ·
      Author Owner

      It's vaguely defined. It's something I want to work on next. Generally PEP8, but it's better to follow the style already in place.

      But it also applies other things like docstrings, that are fairly well done here.

    • Please register or sign in to reply
  • Thomas Kluyver
    Thomas Kluyver @kluyvert started a thread on the diff
    • .gitlab/merge_request_templates/default.md 0 → 100644
    13 ## Relevant Documents (optional)
    14 <!-- Include any relevant screenshot, elogs, reports, if appropriate. -->
    15
    16 ## Types of changes
    17 <!--- What types of changes does your code introduce? Uncomment all lines that apply: -->
    18
    19 <!-- - Bug fix (non-breaking change which fixes an issue) -->
    20 <!-- - New feature (non-breaking change which adds functionality) -->
    21 <!-- - Breaking change (fix or feature that would cause existing functionality to not work as expected) -->
    22
    23 ## Checklist:
    24 <!--- Go over all the following points, and uncomment all lines that apply: -->
    25
    26 <!-- - My code follows the code style of this project. -->
    27 <!-- - My change requires a change to the documentation. -->
    28 <!-- - I have updated the documentation accordingly. -->
  • Karim Ahmed
    Karim Ahmed @ahmedk ·
    Owner

    CC @roscar @hammerd

  • Robert Rosca
    Robert Rosca @roscar ·
    Owner

    LGTM :thumbsup:

  • Robert Rosca
    Robert Rosca @roscar ·
    Owner

    Hey @danilevc you've got 2 LGTM's now, wanna press the big green button :wink:

    Edited by Robert Rosca
  • Cyril Danilevski
    Cyril Danilevski @danilevc ·
    Author Owner

    Thanks for everyone's feedback, we converged towards something cool to use :)

  • Cyril Danilevski mentioned in commit baff7e0e

    mentioned in commit baff7e0e

  • Cyril Danilevski merged

    merged

    • Please register or sign in to reply

    Legal Notice | Contacts