Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Draft: allow searching MRs by a state when it makes sense #623

Merged
merged 18 commits into from
Mar 5, 2021
Merged

Draft: allow searching MRs by a state when it makes sense #623

merged 18 commits into from
Mar 5, 2021

Conversation

maxice8
Copy link
Collaborator

@maxice8 maxice8 commented Feb 18, 2021

Description

This allows us to filter branch names by their state, allowing us to do certain things:

  • Search only "opened" merge requests for mr close, mr merge, mr rebase and others
  • Search only "closed" merge requests for mr reopen
  • Search merge requests in any state if they do not have a compelling reason to be able to restrict the state

This change will increase the amount of prompt that glab does, if lots of people in your project submits merge requests via the name master or main (or whatever people use these days) and you want to interact with a specific one you will be endlessly prompted.

Related Issue

Resolves #620

How Has This Been Tested?

See screenshots

Screenshots (if appropriate):
image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation
  • Chore (Related to CI or Packaging to platforms)

@maxice8 maxice8 changed the title More than one but less Draft: More than one but less Feb 18, 2021
@maxice8 maxice8 marked this pull request as draft February 18, 2021 01:39
@maxice8
Copy link
Collaborator Author

maxice8 commented Feb 18, 2021

Marked as Draft: because it is built on top of #622 and that one needs to be merge and this one rebased

this allows us to filter which merge requests we want to search when
searching by branch name, to also consider the state

the previous default was to search only with the state "opened"

this makes no sense for certain subcommands like 'reopen' whose entire
purpose is purpose it to change the state of a merge request from
"closed" to "reopened", as a consequence the 'reopen' subcommand would
be absolutely useless when dealing with branch names instead of MR-IIDs

this also applies to a lesser extent to other commands like 'note',
there are valid reasons to comment on merge requests that have been
"merged" or "closed", like thanking the contributor for the contribution
this quick-fixes the tests to conform to the changes done in the
previous commit

unfortunately it does not add more tests to see how well it handles the
state
it makes sense that we should be able to view changes in "opened",
"closed", "merged" and "locked" merge requests so use the keyword "any"
it makes sense to be able to update merge requests that are in any state
it makes sense to be able to sub and unsub from merge requests whatever
state they are in
GitLab only allows approving and revoking approval when a merge request
is "opened"
it is only possible to "reopen" merge requests that have been "closed"
we can only rebase "opened" merge requests
we should be able to comment on any merge request, except ones that are
"locked" and we do not have high enugh permissions to comment despite it
being "locked"

unfortunately there is no implemented way in GitLab to query that so
just search for any merge request
we can only merge "opened" merge requests
we can only close merge requests that are "opened"
we can checkout the changes in any merge request, in the future we might
want to restrict "merged" ones when we have the capabilities
@maxice8 maxice8 changed the title Draft: More than one but less Draft: allow searching MRs by a state when it makes sense Feb 22, 2021
@maxice8
Copy link
Collaborator Author

maxice8 commented Feb 22, 2021

Rebased this on top of trunk, it should be good to open to review

@maxice8 maxice8 marked this pull request as ready for review February 22, 2021 01:40
@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

Merging #623 (dc16b13) into trunk (c143c98) will increase coverage by 0.01%.
The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk     #623      +/-   ##
==========================================
+ Coverage   60.51%   60.52%   +0.01%     
==========================================
  Files          90       90              
  Lines        6357     6359       +2     
==========================================
+ Hits         3847     3849       +2     
  Misses       2148     2148              
  Partials      362      362              
Impacted Files Coverage Δ
commands/mr/mrutils/mrutils.go 79.83% <80.00%> (+0.33%) ⬆️
commands/mr/delete/mr_delete.go 84.61% <100.00%> (ø)
commands/mr/diff/diff.go 84.21% <100.00%> (ø)
commands/mr/note/mr_note_create.go 93.33% <100.00%> (ø)
commands/mr/subscribe/mr_subscribe.go 81.25% <100.00%> (ø)
commands/mr/unsubscribe/mr_unsubscribe.go 81.25% <100.00%> (ø)
commands/mr/view/mr_view.go 75.35% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c143c98...dc16b13. Read the comment docs.

Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@zemzale zemzale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxice8 maxice8 merged commit 3f678dd into profclems:trunk Mar 5, 2021
@maxice8 maxice8 deleted the more-than-one-but-less branch March 5, 2021 14:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mr reopen doesn't search closed MRs
3 participants