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

Include more details to mr view #585

Merged
merged 2 commits into from
Jan 26, 2021
Merged

Include more details to mr view #585

merged 2 commits into from
Jan 26, 2021

Conversation

profclems
Copy link
Owner

@profclems profclems commented Jan 23, 2021

Description

  • feat(commands/mr/view): add pipeline status
  • Include more details to MR view

Related Issue

Resolves #557, #322

How Has This Been Tested?

Screenshots (if appropriate):

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)

@profclems profclems added cmd: mr Issue is related to merge request management enhancement New feature or request labels Jan 23, 2021
@profclems profclems linked an issue Jan 23, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Jan 23, 2021

Codecov Report

Merging #585 (2f4e7f3) into trunk (8f5df7f) will decrease coverage by 0.12%.
The diff coverage is 5.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk     #585      +/-   ##
==========================================
- Coverage   54.76%   54.63%   -0.13%     
==========================================
  Files         103      103              
  Lines        6928     6946      +18     
==========================================
+ Hits         3794     3795       +1     
- Misses       2777     2791      +14     
- Partials      357      360       +3     
Impacted Files Coverage Δ
commands/mr/view/mr_view.go 74.63% <5.55%> (-10.37%) ⬇️

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 8f5df7f...2f4e7f3. Read the comment docs.

var status string
switch s := mr.Pipeline.Status; s {
case "failed":
status = utils.Red(s)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We had a change for ci status, that we printed pipeline status yellow, if it's allowed to fail
like here

case "failed":
if job.AllowFailure {
status = utils.Yellow(s)
} else {
status = utils.Red(s)
}

I would actually suggest that we bring out this pipeline status printing out, it's own helper function, so we can centralize it everywhere.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah you're right. I will do that

@profclems
Copy link
Owner Author

@zemzale do you have any suggestion as to how we can implement the approval status?

The Approvals API is only available for starter and bronze users so I'm hesitant to use it.

@zemzale
Copy link
Collaborator

zemzale commented Jan 25, 2021

I don't have any experience with the approval API or even that part of MRs, since I don't use it.

Saddly I am currently not really able to dig deeper into the topic either. So I cant help on this one

@profclems
Copy link
Owner Author

I think the approval status can't be part of v1.14.0 yet

@profclems profclems merged commit d1770b5 into trunk Jan 26, 2021
@profclems profclems deleted the feat-view-details branch January 26, 2021 22:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cmd: mr Issue is related to merge request management enhancement New feature or request size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

show pipeline status of the merge request Include more details in glab mr view
2 participants