RFR: 1258: Add review link/details to comments section
Guoxiong Li
gli at openjdk.java.net
Fri Dec 3 17:20:17 UTC 2021
On Fri, 3 Dec 2021 15:16:36 GMT, Guoxiong Li <gli at openjdk.org> wrote:
> If you look in JiraIssue::addWebLink, it already uses a comment instead of a link in certain cases (specifically when the link is considered confidential). This is the behavior we now wish to emulate all the time, but in addition to the weblink. IMO, we just need to change this method a bit, so that it always calls ::addWebLinkAsComment and then skips the weblink part if needsSecurity is true.
@erikj79 I re-read the code. If we make `addWebLinkAsComment` always be invoked, it may cause some unexpected behavior.
For example, when a patch integrates, a link will be added to the issue (The code is at `IssueNotifier#onIntegratedPullRequest`). At the same time, a comment will be added to the issue (The code is at `IssueNotifier#onNewCommits`). If we enable the `addWebLinkAsComment`, two similar comments would be added to the same issue.
We can remove the related code in `IssueNotifier#onNewCommits` to let the bot only add one comment. But actually, the comment message of the `IssueNotifier#onNewCommits` is more readable and precise. If we always remove the related code like `IssueNotifier#onNewCommits`, I can't find all the places to modify now and the change is more large and more difficult.
So I advice that we can add a warning comment in the api `removeComment` so that the developers use this api carefully.
> Also, if you add new API calls to various REST providers, those need thorough testing which can be hard to perform. We unfortunately do not have any automated way of testing the interaction between Skara and Github/Gitlab/Jira.
If you agree to add the api `removeComment`, this patch needs to be reviewed carefully according to the documentation of the `Github/Gitlab/Jira`.
----
When I type this comment, I receive the your new comment just now. If something is duplicated, just ignore it.
-------------
PR: https://git.openjdk.java.net/skara/pull/1250
More information about the skara-dev
mailing list