ndimiduk commented on code in PR #261: URL: https://github.com/apache/yetus/pull/261#discussion_r860765482
########## asf-site-src/source/documentation/in-progress/precommit/plugins/github.html.md: ########## @@ -41,8 +41,16 @@ None | `--github-api-url=<url>` | REST API URL (for GitHub Enterprise) | | `--github-base-url=<url>` | Non-REST API URL (for GitHub Enterprise) | | `--github-repo=<repo>` | `username/repository` identifier | +| `--github-status-use-htmlreport=<bool>` | Use htmlout for Github Status 'Details' link | | `--github-token=<token>` | Token used to perform read and write operations | +## HTML Details LInk + +By default, if Apache Yetus is under conditions when it would normally write a Github Status (e.g., Jenkins processing Review Comment: Can you show an example of the current behavior and (if possible) the htmlreport that will be the new default after this patch? I'm looking at an [HBase PR](https://github.com/apache/hbase/pull/4305) (Yetus running in the context of an Jenkins Multi-branch plugin) and I don't see a "Details" link anywhere in the comment left by the Jenkins build worker. Perhaps my example context doesn't apply to this change? ########## precommit/src/main/shell/plugins.d/github.sh: ########## @@ -1143,6 +1179,7 @@ function github_finalreport # - failure w/log # - success w/warning log + url=$(get_artifact_url) Review Comment: Minor nit: why move population of the `url` value? Does the `artifact_url` change from the beginning of this function call to down here? ########## precommit/src/main/shell/plugins.d/htmlout.sh: ########## @@ -283,3 +297,39 @@ function htmlout_finalreport printf "<p>This message was automatically generated.</p>" >> "${commentfile}" } + + +## @description Write out an HTML version of the final report to a file +## @audience private +## @stability evolving +## @replaceable no +## @param runresult +function htmlout_finalreport +{ + declare result=$1 + + rm "${commentfile}" 2>/dev/null Review Comment: Should you check the exist code of `rm` and handle failure? ########## precommit/src/main/shell/plugins.d/htmlout.sh: ########## @@ -283,3 +297,39 @@ function htmlout_finalreport printf "<p>This message was automatically generated.</p>" >> "${commentfile}" } + + +## @description Write out an HTML version of the final report to a file +## @audience private +## @stability evolving +## @replaceable no +## @param runresult +function htmlout_finalreport +{ + declare result=$1 + + rm "${commentfile}" 2>/dev/null Review Comment: Why `rm` with error output ignored instead of something like `rm -f` ? ########## asf-site-src/source/documentation/in-progress/precommit/plugins/github.html.md: ########## @@ -41,8 +41,16 @@ None | `--github-api-url=<url>` | REST API URL (for GitHub Enterprise) | | `--github-base-url=<url>` | Non-REST API URL (for GitHub Enterprise) | | `--github-repo=<repo>` | `username/repository` identifier | +| `--github-status-use-htmlreport=<bool>` | Use htmlout for Github Status 'Details' link | | `--github-token=<token>` | Token used to perform read and write operations | +## HTML Details LInk + +By default, if Apache Yetus is under conditions when it would normally write a Github Status (e.g., Jenkins processing Review Comment: Or perhaps my sample PR doesn't apply at all because HBase isn't yet using the GitHub Checks API... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
