findepi commented on code in PR #4989:
URL: https://github.com/apache/iceberg/pull/4989#discussion_r892015058


##########
.github/workflows/api-binary-compatibility.yml:
##########
@@ -38,15 +38,25 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
+        if: github.event_name == 'pull_request'
+        with:
+          # fetch-depth of zero ensures that the tags are pulled in and we're 
not in a detached HEAD state
+          # revapi depends on the tags, specifically the tag from git 
describe, to find the relevant override
+          # in the .palantir/revapi.yml file
+          #
+          # See https://github.com/actions/checkout/issues/124
+          fetch-depth: 0
+          ref: ${{ github.event.pull_request.head.ref }}

Review Comment:
   the point is -- i don't know why you want to specify `ref` at all, instead 
of relying on checkout action's default behavior.
   if you did go with defaults, you wouldn't need these `if`s, so it would be 
simpler, so there must be a reason not to.
   IMO the reason should be documented as a code-level comment (unless it's 
obvious, which doesn't seem to be the case)



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to