Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cml-send-comment --update #581

Merged
merged 15 commits into from
Jun 22, 2021
Merged

Add cml-send-comment --update #581

merged 15 commits into from
Jun 22, 2021

Conversation

0x2b3bfa0
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 commented Jun 8, 2021

Running cml-send-comment with the newly introduced --update flag will update the last CML comment instead of creating a new one. Comment authorship is determined by checking an invisible watermark [sic] added by CML to every machine-made comment. Watermarks are just void images appended with Markdown at the end of comments.

Supported platforms

  • GitHub
  • GitLab1
  • Bitbucket2

1 Blocked by gitlab-org/gitlab#333000; there is no known workaround.
2 Requires user interaction for the first time, as per #581 (comment).

@0x2b3bfa0 0x2b3bfa0 added enhancement New feature or request cml-comment Subcommand blocked Dependent on something else labels Jun 8, 2021
@0x2b3bfa0 0x2b3bfa0 self-assigned this Jun 8, 2021
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 8, 2021 15:27 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 8, 2021 15:28 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 8, 2021 16:52 Inactive
@0x2b3bfa0 0x2b3bfa0 linked an issue Jun 9, 2021 that may be closed by this pull request
@casperdcl
Copy link
Contributor

Given https://github.com/iterative/cml/wiki/Backend-Supported-Features we could still merge anyway

@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Jun 15, 2021

Pull Request Commit Links app must be installed first before using this API; installation automatically occurs when 'Go to pull request' is clicked from the web interface for a commit's details. (Clinical definition of insanity)

⚠️ Bitbucket support won't be fully functional until users (cough) install the Pull Request Commit Links application. In the worst case scenario, commit comments will be created and pull request comments won't.

@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 15, 2021 23:38 Inactive
@0x2b3bfa0 0x2b3bfa0 removed the blocked Dependent on something else label Jun 15, 2021
bin/cml-send-comment.js Outdated Show resolved Hide resolved
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 15, 2021 23:52 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 15, 2021 23:55 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 16, 2021 00:05 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 16, 2021 00:16 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 16, 2021 00:32 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 16, 2021 00:38 Inactive
@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Jun 16, 2021

Hopefully I won't have to think about John Woods' quote for a long, long time. 😈

bin/cml-send-comment.js Show resolved Hide resolved
bin/cml-send-comment.js Show resolved Hide resolved
src/drivers/bitbucket_cloud.js Outdated Show resolved Hide resolved
src/drivers/github.test.js Show resolved Hide resolved
Copy link
Contributor

@DavidGOrtega DavidGOrtega left a comment

Choose a reason for hiding this comment

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

⚠️ Bitbucket support won't be fully functional until users (cough) install the Pull Request Commit Links application. In the worst case scenario, commit comments will be created and pull request comments won't.

I wonder if we should unsupport Bitbucket. Sounds very flaky and confusing

@0x2b3bfa0
Copy link
Member Author

What if we convert the disabled API warning into a fatal error and put it first? This way, users will need to enable the API before using cml-send-comment on Bitbucket. Nevertheless, it's as confusing as you think and, even with good documentation, may be challenging for some users.

@DavidGOrtega
Copy link
Contributor

...it's as confusing as you think and, even with good documentation, may be challenging for some users.

Let's remove it then for Bitbucket.

@casperdcl
Copy link
Contributor

😱 I wouldn't remove BB functionality just because of a confusing one-click mechanism. Helpful error message (possibly throw/non-zero exit) plz.

@DavidGOrtega
Copy link
Contributor

😱 I wouldn't remove BB functionality just because of a confusing one-click mechanism. Helpful error message (possibly throw/non-zero exit) plz.

Ok but also has to be well documented, the last thing I want is we having to answer questions about why is not working in BB

@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 18, 2021 13:12 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 18, 2021 13:14 Inactive
@0x2b3bfa0
Copy link
Member Author

@DavidGOrtega & @casperdcl, do you like this fix à la #563ème?

@casperdcl casperdcl temporarily deployed to internal June 18, 2021 16:53 Inactive
@casperdcl casperdcl temporarily deployed to internal June 22, 2021 21:41 Inactive
@casperdcl casperdcl temporarily deployed to internal June 22, 2021 21:47 Inactive
@casperdcl casperdcl temporarily deployed to internal June 22, 2021 21:53 Inactive
@0x2b3bfa0
Copy link
Member Author

I miss #538 with granular tests and ephemeral repositories. 🐲 Having to manually delete comments before running automated tests is... far from ideal.

FAIL src/drivers/bitbucket_cloud.test.js
[Forbidden] By default, you can't create more than 1000 comments per pull request. Contact support to update this limit.

@casperdcl casperdcl temporarily deployed to internal June 22, 2021 21:56 Inactive
@0x2b3bfa0 0x2b3bfa0 merged commit cf0ba10 into master Jun 22, 2021
@0x2b3bfa0 0x2b3bfa0 deleted the cml-send-comment-update branch June 22, 2021 22:39
@DavidGOrtega
Copy link
Contributor

I miss #538 with granular tests and ephemeral repositories. 🐲 Having to manually delete comments before running automated tests is... far from ideal.

FAIL src/drivers/bitbucket_cloud.test.js
[Forbidden] By default, you can't create more than 1000 comments per pull request. Contact support to update this

This is weird, we fixed this already. As far as I remmber

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cml-comment Subcommand enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add cml-send-comment --update
4 participants