Review with quick actions

[zampierilucas]

This is the second part of the change to unify the calls made when approving/commenting on an MR.

The new code removes the usage of the merge_request/approve API and instead uses the /approve and /unapprove notes API quick actions.
Fixes: #708

[bmeneg]

Thanks @zampierilucas :)

[zampierilucas]

I found an issue when using --with-comment

[zampierilucas]

Submitted a new commit with a fix to --with-comment and everything should be working fine.

[bmeneg]

@zampierilucas I don't think the build issue you're facing is really related to your PR.
I just hit during the release testing: https://app.travis-ci.com/github/zaquestion/lab/builds/238658259

[zampierilucas]

@zampierilucas I don't think the build issue you're facing is really related to your PR. I just hit during the release testing: https://app.travis-ci.com/github/zaquestion/lab/builds/238658259

@bmeneguele GREAT!! I mean bad, really bad :p
but you're right mr_note_test.go:102 is the same one that is breaking on my MR, maybe it has been broken for a while?
OTOH I've tested the command that the it sends lab mr reply origin 1:689060304 -m "text" and It succeedss without a problem, so are we having some issue with our testing <-> gitlab.com relation again?

[bmeneg]

Possibly, but it shouldn't be panic'ing, but just dropping an error of any sort.
I'll try to take a look real quick to see if I can figure out and get the release out of the door.
However, if we can get it solved for the release, we can also get your PR also included before releasing :)

[bmeneg]

The issue is related to something on gitlab side. Two commands in the very same MR, but in different notes behaves differently with no apparent reason:

$ lab mr reply lab-testing 3953:599577526 -m "test"
https://gitlab.com/lab-testing/test/merge_requests/3953#note_689563923
$ lab mr reply lab-testing 3953:689488483 -m "test"
$ 

The first run returns what is expected, in the last one, it doesn't.
For "old" notes, it seems to work just fine, while for new ones, it doesn't.

Something changed in the API?
I'm going to dig a further.

[bmeneg]

For some really weird reason the discussions API is not returning new notes added to the MR, no matter if it's created using the Notes (where individual_note = true) or the Discussion API (where individual_note = false).

I tested it using the API directly (through curl) so that's not an issue with lab, but definitely an issue on GitLab itself and a recent one, because the last PR that @prarit created was 4 days ago and everything was working just fine. I didn't find any issue open on gitlab.org though.

[bmeneg]

Well, the problem turned out to be a lack of an HTTP header in API responses for MRs/Issues with more than 10k comments, which happens in our testing repo.

@zampierilucas helped me to find the issue and the PR #745 was created with the fix.

[bmeneg]

As soon as #745 is merged, this PR will need to be rebased.

[prarit]

10k? What have you been doing to my MRs? 😄

[zampierilucas]

@bmeneguele sure, I will rebase it as soon as 745 gets merged :D

10k? What have you been doing to my MRs? smile

@prarit GL considers 'entries' as any historical data on the MR, not only 'comments', for example, 'X person approved the MR' is also an entry, That's why we were able to beet GitLab 10k restriction that fast :p

[prarit]

Entries? I'm just going to assume that somewhere there is a character-by-character review of one of my changesets.

[bmeneg]

@zampierilucas #745 is now merged, please go ahead and rebase this one here so we can make sure it's also fine :)

[codecov]

Codecov Report

Merging #741 (8bbc211) into master (2fb6734) will increase coverage by 0.04%.
The diff coverage is 45.00%.

@@            Coverage Diff             @@
##           master     #741      +/-   ##
==========================================
+ Coverage   54.71%   54.75%   +0.03%     
==========================================
  Files          77       77              
  Lines        5596     5612      +16     
==========================================
+ Hits         3062     3073      +11     
- Misses       2251     2255       +4     
- Partials      283      284       +1     

Impacted Files	Coverage Δ
cmd/mr_approve.go	70.27% <33.33%> (-1.61%)	⬇️
cmd/mr_unapprove.go	75.00% <47.36%> (+3.12%)	⬆️
cmd/note_common.go	54.57% <66.66%> (-0.24%)	⬇️

---

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 2fb6734...8bbc211. Read the comment docs.