kbendick commented on PR #4989: URL: https://github.com/apache/iceberg/pull/4989#issuecomment-1150262358
> Thanks @kbendick for the change and comprehensive deep-dive, the above approach LGTM !! > > [question] : Have a small question, based on this [article](https://frontside.com/blog/2020-05-26-github-actions-pull_request/#how-does-pull_request-affect-actionscheckout) > > > that a pull_request workflow `ref` would look like refs/remotes/pull/##/merge, SHA of a pull_request workflow doesn't matches the commit that triggered the workflow; instead the SHA of the pull_request is the resulting commit that was created from merging the base to the head. > > Are we ok failing in the CI incase there is a merge conflict when merging base to the head ? Thoughts. (I think we should be ok, considering we need to resolve them any how before final merge.) So if I understand the article correctly, we _might _ still need to use the specific branch for `pull_request`. I tested in my fork without it, and it was fine. Additionally, the article states that in the worse case we'll get false positives. I'd rather that than false negatives as mentioned. Let's go with this simpler workflow for now as I've tested it extensively in my fork and if we get false positives - then we'll catch them and deal with it then. But thanks for the article link, it's very helpful. Same applies for false negatives which we _should_ catch eventually as people run via the CLI which should always generate the proper result. -- 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]
