Skip to content
Snippets Groups Projects

Document the role of the maintainer

Merged Douwe Maan requested to merge dm-document-role-maintainer into master

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
  • @DouweM LGTM, just one fix.

  • 3 Errors
    :no_entry_sign: 2a631de5: The commit subject must not end with a period
    :no_entry_sign: 90056ed2: The commit subject must not end with a period
    :no_entry_sign: eb0ded1d: The commit subject may not be longer than 72 characters
    5 Warnings
    :warning: This merge request does not refer to an existing milestone.
    :warning: CHANGELOG missing.

    You can create one with:

    bin/changelog -m 22232 "Document the role of the maintainer"

    If your merge request doesn’t warrant a CHANGELOG entry,
    consider adding any of the ~backstage, ~Documentation, QA, test labels.
    See the documentation.

    :warning: This merge request is missing the ~Documentation label.
    :warning: 2a631de5: This commit’s subject line could be improved. Commit subjects are ideally no longer than roughly 50 characters, though we allow up to 72 characters in the subject. If possible, try to reduce the length of the subject to roughly 50 characters.
    :warning: 90056ed2: This commit’s subject line could be improved. Commit subjects are ideally no longer than roughly 50 characters, though we allow up to 72 characters in the subject. If possible, try to reduce the length of the subject to roughly 50 characters.
    1 Message
    :book: This merge request adds or changes files that require a review from the docs team.

    Docs Review

    The following files require a review from the Documentation team:

    • doc/development/code_review.md

    To make sure these changes are reviewed, mention @gl-docsteam in a separate comment, and explain what needs to be reviewed by the team. Please don't mention the team until your changes are ready for review.

    Commit message standards

    One or more commit messages do not meet our Git commit message standards. For more information on how to write a good commit message, take a look at How to Write a Git Commit Message.

    Here is an example of a good commit message:

    Reject ruby interpolation in externalized strings
    
    When using ruby interpolation in externalized strings, they can't be
    detected. Which means they will never be presented to be translated.
    
    To mix variables into translations we need to use `sprintf`
    instead.
    
    Instead of:
    
        _("Hello #{subject}")
    
    Use:
    
        _("Hello %{subject}") % { subject: 'world' }

    This is an example of a bad commit message:

    updated README.md

    This commit message is bad because although it tells us that README.md is updated, it doesn't tell us why or how it was updated.

    Generated by :no_entry_sign: Danger

    Edited by 🤖 GitLab Bot 🤖
  • Hi @DouweM,

    Please add labels to your merge request, this helps triage community merge requests.

    Thanks for your help! :black_heart:


    You are welcome to help improve this comment.

  • Douwe Maan added 1 commit

    added 1 commit

    • eb0ded1d - Rewrite guidance on getting your merge request reviewed, approved, and merged

    Compare with previous version

  • Douwe Maan
  • Douwe Maan
  • closed

  • reopened

  • assigned to @smcgivern

  • Fatih Acet
  • Fatih Acet
  • Nick Thomas
  • Sean McGivern
  • Sean McGivern
  • Andreas Brandl
  • Rémy Coutable
  • Rémy Coutable
  • Rémy Coutable
  • Thanks for this @DouweM! The only remark I have is the same as Nick's

  • assigned to @DouweM

  • Douwe Maan added 1 commit

    added 1 commit

    • 90056ed2 - Clarify responsibilities of MR author and maintainer based on feedback.

    Compare with previous version

  • Douwe Maan unmarked as a Work In Progress

    unmarked as a Work In Progress

  • assigned to @smcgivern

  • Author Contributor

    I've updated the MR based on the feedback, and marked @nick.thomas, @smcgivern, @grzesiek, @rymai and @filipa as required approvers since the most significant feedback on the issue and the MR came from them.

    Please have a look!

  • Douwe Maan added 236 commits

    added 236 commits

    Compare with previous version

  • Grzegorz Bizon
  • Sean McGivern
  • Sean McGivern approved this merge request

    approved this merge request

  • @DouweM works for me (one minor question aside), thanks! Assigning to the next in the approvals list.

  • assigned to @rymai

  • Filipa Lacerda approved this merge request

    approved this merge request

  • mentioned in issue #52788 (closed)

  • Grzegorz Bizon
  • Nick Thomas approved this merge request

    approved this merge request

  • @DouweM I left just two questions!

  • Rémy Coutable
  • @DouweM LGTM, I only left minor notes, so I'm approving already and passing to you since it seems there are other discussions to be resolved.

  • assigned to @DouweM

  • Rémy Coutable approved this merge request

    approved this merge request

  • Douwe Maan added 1 commit

    added 1 commit

    • 2a631de5 - Strongly recommend involving a domain expert, especially when in doubt.

    Compare with previous version

  • assigned to @grzesiek

  • Author Contributor

    Thanks for your feedback, everyone!

    @grzesiek Please have a look at my attempt at addressing your concerns, and please resolve the discussions and approve and merge the MR if you think it's sufficient :)

  • Grzegorz Bizon resolved all discussions

    resolved all discussions

  • Grzegorz Bizon approved this merge request

    approved this merge request

  • Grzegorz Bizon mentioned in commit 4ef9bb6a

    mentioned in commit 4ef9bb6a

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading