+1 That was the behavior we used to have in Jenkins.
-- Matteo Merli <matteo.me...@gmail.com> On Wed, Mar 17, 2021 at 8:15 AM Michael Marshall <mikemars...@gmail.com> wrote: > > +1 > > I like the idea that this change would give PRs the benefit of any recently > fixed flaky tests without needing to rebase. Rebasing to pick up fixes to > flaky tests slows down development. > > Further, I think this sounds like a safer paradigm for maintaining a > pristine master branch. > > Michael > > On Wed, Mar 17, 2021 at 2:01 AM Enrico Olivelli <eolive...@gmail.com> wrote: > > > Il giorno mer 17 mar 2021 alle ore 08:56 Lari Hotari > > <lari.hot...@sagire.fi> ha scritto: > > > > > > Dear Pulsar community members, > > > > > > I would like to propose a change how Pull Request (PR) builds are handled > > > in Pulsar CI. > > > > > > In GitHub Actions, the default way to handle PR builds is to checkout > > > GitHub provided merge_commit_sha . > > > > > > GitHub's merge_commit_sha is explained in this documentation: > > > https://docs.github.com/en/rest/reference/pulls#get-a-pull-request > > > "Before merging a pull request, the merge_commit_sha attribute holds the > > > SHA of the test merge commit." > > > Behind the scenes, GitHub automatically creates a "test merge commit" > > that > > > merges the PR branch and the target branch (master). A pull request build > > > won't be started at all if this test merge commit cannot be created > > without > > > conflicts. > > > > > > In Pulsar GitHub Actions workflows, the PR builds happen for the HEAD > > > commit on the pull request branch, without a merge to the master branch. > > > > > > *The downside of the approach used in the existing Pulsar CI is that the > > PR > > > build won't pick up changes made in the master branch.* It is required to > > > explicitly rebase or merge the master branch to the pull request branch > > to > > > get the changes. Another disadvantage is that there's a higher chance > > that > > > the PR breaks the master branch after the PR is merged. > > > The isolated behavior might be seen as a benefit. The PR branch build > > will > > > be only impacted by the changes made in the PR branch. > > > However, using this type of setting isn't *Continuous* Integration (CI). > > > > > > The current custom build "diff-only" action ( > > > https://github.com/apache/pulsar-test-infra/tree/master/diff-only) > > doesn't > > > support merge_commit_sha . When switching to use merge_commit_sha instead > > > of PR's head commit for git checkout, I'm suggesting to replace diff-only > > > action with https://github.com/dorny/paths-filter action . paths-filter > > > action doesn't have the limitations of diff-only action and uses GitHub > > API > > > for resolving the changed files without a need to doing a checkout. It > > is a > > > very high quality GitHub Action for adding conditional logic to Github > > > Action workflows based on the files that have changed. > > > > > > What do others think about making this change, using GitHub's > > > merge_commit_sha for PR builds instead of PR's HEAD commit? > > > > +1 > > using the PR HEAD commit is dangerous as we believe that the PR is not > > breaking the target branch but currently we have no proof of that. > > The risk is PRs that are far from the target branch. > > When you merge a PR you don't have an easy way to check how far it is > > from the target branch and you usually verify only that CI is green > > and there are no conflicts > > > > Enrico > > > > > > > > BR, Lari > >
