Skip to content
Snippets Groups Projects

Fix issues mentioned but not closed for JIRA

Merged Sean McGivern requested to merge fix-mentioned-issues-for-external-trackers into master
All threads resolved!

What does this MR do?

When using an external issue tracker, we would show issues that were closed by an MR as both:

  • Accepting this merge request will close issue X.
  • Issue X is mentioned but will not be closed.

Are there points in the code the reviewer needs to double check?

I feel a little uncertain about overriding ExternalIssue#hash, but it seems the simplest solution.

Why was this MR needed?

The bug was really dumb.

Screenshots (if relevant)

image

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/26028.

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
  • Adam Niedzielski
  • @smcgivern I think that it's the right approach. http://ruby-doc.org/core-2.4.0/Object.html#method-i-eql-3F says that:

    Subclasses normally continue this tradition by aliasing eql? to their overridden == method

    I think that it's totally fine to override eql? and hash here.

    I left two minor comments and everything else looks good to me.

  • Sean McGivern resolved all discussions

    resolved all discussions

  • Author Contributor

    @adamniedzielski thanks, updated!

  • Sean McGivern added 2 commits

    added 2 commits

    • 9f612cc4 - Fix issues mentioned but not closed for JIRA
    • 5ce3845f - DRY up {jira,kubernets}_project factories

    Compare with previous version

  • @smcgivern Everything looks fine to me now.

  • Sean McGivern enabled an automatic merge when the pipeline for 5ce3845f succeeds

    enabled an automatic merge when the pipeline for 5ce3845f succeeds

  • merged

  • @smcgivern Is there any chance, that this could go into version 8.17.3?

  • Author Contributor

    @jannickfahlbusch I'm afraid not, it will be in 9.0 :disappointed:

  • Please register or sign in to reply
    Loading