Skip to content
Snippets Groups Projects

Merge Request Discussions API: accept commit_id to create discussion on commit

What does this MR do?

When creating a new thread on a merge request by using the MR Discussions API, the thread is always on the merge request's entire diff.

This patch allows passing the commit_id parameter to associate the thread with the desired commit.

Here is a screenshot of the results. Both comments were created using the API. The second comment was created after my changes, and thus contains the desired started a thread on commit 555c8706.

Before

old

After

new

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Johannes Altmanninger

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
  • added 2957 commits

    • f547c0ae...44b5330e - 2956 commits from branch gitlab-org:master
    • 53c824ec - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • Johannes Altmanninger marked the checklist item Changelog entry as completed

    marked the checklist item Changelog entry as completed

  • added 1 commit

    • e168f727 - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • assigned to @krobelus

  • added 1 commit

    • 936f2d71 - Apply 2 suggestion(s) to 2 file(s)

    Compare with previous version

  • added 1 commit

    • 43b64af7 - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • added 1 commit

    • ec7aa715 - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • added 1 commit

    • 25ba9589 - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • added 1 commit

    • a8aabba5 - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • added 2606 commits

    • a8aabba5...58bc9bea - 2605 commits from branch gitlab-org:master
    • 770cb3ab - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • Patrick Bajao approved this merge request

    approved this merge request

  • assigned to @kerrizor and @marcia

  • Johannes Altmanninger resolved all threads

    resolved all threads

  • added 1 commit

    • 7552982a - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Kerri Miller approved this merge request

    approved this merge request

  • added 1 commit

    • 8b6b7796 - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • Johannes Altmanninger resolved all threads

    resolved all threads

  • Marcia Ramos
  • unassigned @marcia

  • Marcia Ramos approved this merge request

    approved this merge request

  • Johannes Altmanninger resolved all threads

    resolved all threads

  • added 1 commit

    • 8b4a6ce6 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • added 1 commit

    • b7f2365f - Merge Request Discussions API: accept commit_id to create discussions on commit

    Compare with previous version

  • Johannes Altmanninger resolved all threads

    resolved all threads

  • Kerri Miller resolved all threads

    resolved all threads

  • 2 Errors
    :no_entry_sign: b7f2365f: The commit subject may not be longer than 72 characters. For more information, take a look at our Commit message guidelines.
    :no_entry_sign: b7f2365f: The commit body should not contain more than 72 characters per line. For more information, take a look at our Commit message guidelines.
    1 Warning
    :warning: This merge request does not refer to an existing milestone.
    1 Message
    :book: This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge.

    Documentation review

    The following files require a review from a technical writer:

    • doc/api/discussions.md

    The review does not need to block merging this merge request. See the:

    Reviewer roulette

    Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.

    To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited, or the chosen person is unavailable.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.

    Category Reviewer Maintainer
    backend Michał Zając (@quintasan) (UTC+1) Imre Farkas (@ifarkas) (UTC+1)

    If needed, you can retry the danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

    Edited by 🤖 GitLab Bot 🤖
  • Kerri Miller enabled an automatic merge when the pipeline for b783fac1 succeeds

    enabled an automatic merge when the pipeline for b783fac1 succeeds

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