Support delete comment

[wandersoncferreira]

Currently we only support deleting a local comment but would be great to support deleting comments that were already sent to Github/Gitlab.

• [ ] [GitHub GraphQL API] https://docs.github.com/pt/graphql/reference/mutations#deleteissuecomment
• [ ] [Gitlab] TBD

Basically the path through this would be to:

1. Create a def-generic function in code-review-interfaces.el
2. Implement the function in code-review-github.el and code-review-gitlab.el
3. Implement an interactive function like this https://github.com/wandersoncferreira/code-review/blob/13a99a2ce9801141462bf425aa40dd2a732eb3e7/code-review-section.el#L1574
4. You can see that the function above is bound here https://github.com/wandersoncferreira/code-review/blob/13a99a2ce9801141462bf425aa40dd2a732eb3e7/code-review-section.el#L548 but we should bind the new one to here https://github.com/wandersoncferreira/code-review/blob/13a99a2ce9801141462bf425aa40dd2a732eb3e7/code-review-section.el#L461

This is a great entry point for someone interesting in becoming familiar with the project. Several other features have similar workflow to be included.

[wandersoncferreira]

@fuzzycode interested?

[fuzzycode]

I think you might overestimate my elisp skills a bit. I tried to have a look at it but I think it is outside of what I will be able to contribute given the amount of time I have available.

It also appears a bit more complex than described as it would not make for good UX if there was 2 different functions, one for deleting local comments and one for deleting comments on GitHub/Gitlab and the user had to know which one to use. There would need to be some kind of dwim command that deletes the comment at point in the right way regardless of where it is. Or did I miss something?

[wandersoncferreira]

I think you might overestimate my elisp skills a bit. I tried to have a look at it but I think it is outside of what I will be able to contribute given the amount of time I have available.

take it as a good learning exercise as this feature is not critical =) but feel free to pass on.

It also appears a bit more complex than described as it would not make for good UX if there was 2 different functions, one for deleting local comments and one for deleting comments on GitHub/Gitlab and the user had to know which one to use. There would need to be some kind of dwim command that deletes the comment at point in the right way regardless of where it is. Or did I miss something?

magit-section does the dwim (sort of) under the cover. We have 2 different keymaps code-review-comment-section-map (for comments on Github/Gitlab) and code-review-local-comment-section (for comments only local) and we can bind the same key C-c C-k to two different functions and let Magit-section call the right one when the point is under a specific section.

[fuzzycode]

Ok, so from a user perspective if they were to rebind the deletion to an other key they would have to make sure to rebind both functions in two different maps to maintain the dwim functionality?

I don't think I fully understand the concept of local comments. Are they local while waiting to be published to github/gitlab or are they intended to remain local forever?

[wandersoncferreira]

Ok, so from a user perspective if they were to rebind the deletion to an other key they would have to make sure to rebind both functions in two different maps to maintain the dwim functionality?

Yes, we can help and create a single function if you want to:

(defun code-review-delete-comment-dwim ()
  (interactive)
  (let ((section (magit-current-section)))
    (cond
     ((code-review-local-comment-section-p section)
      (code-review-section-delete-comment))
     ((code-review-comment-section-p section)
      (code-review-NEW-DELETE-COMMENT-fn))
     (t
      (error "Section under point not supported.")))))

I don't think I fully understand the concept of local comments. Are they local while waiting to be published to github/gitlab or are they intended to remain local forever?

They are local while we wait to publish it. However, all the comments are stored in a sqlite database if you call code-review-save-unfinished-review you can flag a specific review in the sqlite database and then recover it later using code-review-open-unfinished-review. The local comments are restored too.

After we publish it, the local comments become "published comments"

[fuzzycode]

I don't think that I know enough to really have a strong opinion at this point, I am just trying to understand how all the things work and fit together :-)

After we publish it, the local comments become "published comments"

I see, so the local part is just a cache of all things waiting to be published.

[wandersoncferreira]

I see, so the local part is just a cache of all things waiting to be published.

Yes, and this is very confusing because some commands do not work like this. For example, if you change the title of a PR does not make sense to have a "local title" and not change upstream immediately (or does it? idk).

So we have things that only change local db (but update the buffer) and other things that changes immediately (and update the buffer)

Basically if you look at the transient menu, all the things there change the review immediately.

[fuzzycode]

This might be a stupid question or me missing something obvious but what are the reasons for not publishing comments immediately like fore example the title?

[wandersoncferreira]

This might be a stupid question or me missing something obvious but what are the reasons for not publishing comments immediately like fore example the title?

its not stupid at all, there are some points to consider:

1. if you start a review process in Github, usually you accumulate your comments and send it with the verdict of the review
2. its harder to mimic the behavior above sending comments directly because you need to send them as "Pending" and them later confirm all of them anyway
3. we avoid the rate limit issues of GitHub API if we send as little requests as possible
4. you can easily remove or type unfinished ideas in the comment to catch up later on
5. I already wanted to have the notion of "unfinished reviews" that I can pickup later and continue

all of these things its easier if you accumulate your diff comments or reply comments locally first before submitting.

[abaransy]

@fuzzycode are you working on this?

[fuzzycode]

Hi @abaransy I am not going to work on this. Swamped with life atm. :-)