You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Enrico Olivelli <eo...@gmail.com> on 2018/07/24 06:41:16 UTC
Precommit Jobs - don't we merge to master before tests ?
Hi,
I am surprised that in our pre-commit jobs on Jenkins we are not merging
the Pull Request Branch with current master before running all tests/checks.
Ideally we have to simulate the final status of the repository after
merging the branch to "master"
This can be simply achieved but adding an "Additional behaviour" on GIT
configuration:
Additional Behaviours - Merge Before build:
Name of repository: origin
Branch to merge to: master
Merge strategy: default
Fast forward mode: --ff
This is the configuration which I am using for all my projects and it works
like a charm
Am I missing something ?
If you agree I will submit a patch to change all the jobs and add this
magic trick
Enrico
Re: Precommit Jobs - don't we merge to master before tests ?
Posted by Enrico Olivelli <eo...@gmail.com>.
Il giorno mar 24 lug 2018 alle ore 13:38 Enrico Olivelli <
eolivelli@gmail.com> ha scritto:
>
>
> Il mar 24 lug 2018, 10:49 Sijie Guo <gu...@gmail.com> ha scritto:
>
>> On Tue, Jul 24, 2018 at 12:32 AM Enrico Olivelli <eo...@gmail.com>
>> wrote:
>>
>> > Il giorno mar 24 lug 2018 alle ore 09:25 Sijie Guo <gu...@gmail.com>
>> ha
>> > scritto:
>> >
>> > > I am not sure need this configuration.
>> > >
>> > > based on my understanding, if we are using `${sha1}`, jenkins will use
>> > > Github's tentative merge of the compare and base branches (e.g.
>> > > refs/pull/<pull-request-id>/merge) if the PR can be automatically
>> merged
>> > or
>> > > the head of the pull request (e.g. refs/pull/<pull-request-id>/head)
>> if
>> > > they can not be automatically merged.
>> > >
>> > > since we only merge a PR that all conflicts are resolved, that means
>> the
>> > > final precommit check status are on the tentative merge of the
>> compare
>> > and
>> > > base branches.
>> > >
>> > > Unless I misunderstood this plugin:
>> > > https://github.com/jenkinsci/ghprb-plugin
>> >
>> >
>> >
>> > Maybe you are right, I am not using that plugin on the Jenkins of my
>> > company.
>> > In Bookkeeper actually we are using ${ghprbActualCommit} and not
>> ${sha1}.
>> >
>>
>> Oh I see. I think I changed it to ${ghprbActualCommit} to support PRs that
>> run against branches.
>>
>>
>> >
>> > I got this idea because you (Sijie) were asking Jia to rebase a PR to
>> > current master in order to suppress a flaky test.
>> >
>>
>> Good point. That's probably due to ${ghprbActualCommit}. ${sha1} probably
>> would address the problem.
>>
>> It would be good to test if ${sha1} works a) does what you want b) can
>> work
>> with branches.
>>
>
>
> I will check docs and eventually try
>
we should use ${sha1} according to this thread on the issue tracker of the
plugin
https://github.com/jenkinsci/ghprb-plugin/issues/563
it should work even for PRs against release (non master) branches
Shall we try ?
Sijie,
Do I need to file a PR for changing the job, or can I change the config
manually and only if it works I will send the change to the config file ?
Enrico
>
>>
>> > Using that "Additional behavior" of Jenkins GIT support I never had such
>> > problem, just re-kicking the job applies current master and picks up all
>> > the improvements.
>> >
>>
>> Agreed. fast-forward merge should be helpful. I think it is no harm to
>> enable that.
>>
>>
>> > It is also safer to have the job run with the branch merged to current
>> > master, because the effect is the same as what the committer would have
>> > done by running tests/QA checks while using the merge script (And I
>> never
>> > do it during the script because it takes too much time).
>> >
>>
>> I think tests/QA checks can be removed from merge script since we are
>> blocking merges if CI doesn't pass.
>>
>
> Sure
>
> Enrico
>
>>
>>
>> >
>> >
>> > Enrico
>> >
>> >
>> >
>> > >
>> > >
>> > > On Mon, Jul 23, 2018 at 11:41 PM Enrico Olivelli <eolivelli@gmail.com
>> >
>> > > wrote:
>> > >
>> > > > Hi,
>> > > > I am surprised that in our pre-commit jobs on Jenkins we are not
>> > merging
>> > > > the Pull Request Branch with current master before running all
>> > > > tests/checks.
>> > > >
>> > > > Ideally we have to simulate the final status of the repository after
>> > > > merging the branch to "master"
>> > > >
>> > > >
>> > > > This can be simply achieved but adding an "Additional behaviour" on
>> GIT
>> > > > configuration:
>> > > >
>> > > > Additional Behaviours - Merge Before build:
>> > > > Name of repository: origin
>> > > > Branch to merge to: master
>> > > > Merge strategy: default
>> > > > Fast forward mode: --ff
>> > > >
>> > > > This is the configuration which I am using for all my projects and
>> it
>> > > works
>> > > > like a charm
>> > > >
>> > > > Am I missing something ?
>> > > > If you agree I will submit a patch to change all the jobs and add
>> this
>> > > > magic trick
>> > > >
>> > > > Enrico
>> > > >
>> > >
>> >
>>
> --
>
>
> -- Enrico Olivelli
>
Re: Precommit Jobs - don't we merge to master before tests ?
Posted by Enrico Olivelli <eo...@gmail.com>.
Il mar 24 lug 2018, 10:49 Sijie Guo <gu...@gmail.com> ha scritto:
> On Tue, Jul 24, 2018 at 12:32 AM Enrico Olivelli <eo...@gmail.com>
> wrote:
>
> > Il giorno mar 24 lug 2018 alle ore 09:25 Sijie Guo <gu...@gmail.com>
> ha
> > scritto:
> >
> > > I am not sure need this configuration.
> > >
> > > based on my understanding, if we are using `${sha1}`, jenkins will use
> > > Github's tentative merge of the compare and base branches (e.g.
> > > refs/pull/<pull-request-id>/merge) if the PR can be automatically
> merged
> > or
> > > the head of the pull request (e.g. refs/pull/<pull-request-id>/head) if
> > > they can not be automatically merged.
> > >
> > > since we only merge a PR that all conflicts are resolved, that means
> the
> > > final precommit check status are on the tentative merge of the compare
> > and
> > > base branches.
> > >
> > > Unless I misunderstood this plugin:
> > > https://github.com/jenkinsci/ghprb-plugin
> >
> >
> >
> > Maybe you are right, I am not using that plugin on the Jenkins of my
> > company.
> > In Bookkeeper actually we are using ${ghprbActualCommit} and not
> ${sha1}.
> >
>
> Oh I see. I think I changed it to ${ghprbActualCommit} to support PRs that
> run against branches.
>
>
> >
> > I got this idea because you (Sijie) were asking Jia to rebase a PR to
> > current master in order to suppress a flaky test.
> >
>
> Good point. That's probably due to ${ghprbActualCommit}. ${sha1} probably
> would address the problem.
>
> It would be good to test if ${sha1} works a) does what you want b) can work
> with branches.
>
I will check docs and eventually try
>
>
> > Using that "Additional behavior" of Jenkins GIT support I never had such
> > problem, just re-kicking the job applies current master and picks up all
> > the improvements.
> >
>
> Agreed. fast-forward merge should be helpful. I think it is no harm to
> enable that.
>
>
> > It is also safer to have the job run with the branch merged to current
> > master, because the effect is the same as what the committer would have
> > done by running tests/QA checks while using the merge script (And I never
> > do it during the script because it takes too much time).
> >
>
> I think tests/QA checks can be removed from merge script since we are
> blocking merges if CI doesn't pass.
>
Sure
Enrico
>
>
> >
> >
> > Enrico
> >
> >
> >
> > >
> > >
> > > On Mon, Jul 23, 2018 at 11:41 PM Enrico Olivelli <eo...@gmail.com>
> > > wrote:
> > >
> > > > Hi,
> > > > I am surprised that in our pre-commit jobs on Jenkins we are not
> > merging
> > > > the Pull Request Branch with current master before running all
> > > > tests/checks.
> > > >
> > > > Ideally we have to simulate the final status of the repository after
> > > > merging the branch to "master"
> > > >
> > > >
> > > > This can be simply achieved but adding an "Additional behaviour" on
> GIT
> > > > configuration:
> > > >
> > > > Additional Behaviours - Merge Before build:
> > > > Name of repository: origin
> > > > Branch to merge to: master
> > > > Merge strategy: default
> > > > Fast forward mode: --ff
> > > >
> > > > This is the configuration which I am using for all my projects and it
> > > works
> > > > like a charm
> > > >
> > > > Am I missing something ?
> > > > If you agree I will submit a patch to change all the jobs and add
> this
> > > > magic trick
> > > >
> > > > Enrico
> > > >
> > >
> >
>
--
-- Enrico Olivelli
Re: Precommit Jobs - don't we merge to master before tests ?
Posted by Sijie Guo <gu...@gmail.com>.
On Tue, Jul 24, 2018 at 12:32 AM Enrico Olivelli <eo...@gmail.com>
wrote:
> Il giorno mar 24 lug 2018 alle ore 09:25 Sijie Guo <gu...@gmail.com> ha
> scritto:
>
> > I am not sure need this configuration.
> >
> > based on my understanding, if we are using `${sha1}`, jenkins will use
> > Github's tentative merge of the compare and base branches (e.g.
> > refs/pull/<pull-request-id>/merge) if the PR can be automatically merged
> or
> > the head of the pull request (e.g. refs/pull/<pull-request-id>/head) if
> > they can not be automatically merged.
> >
> > since we only merge a PR that all conflicts are resolved, that means the
> > final precommit check status are on the tentative merge of the compare
> and
> > base branches.
> >
> > Unless I misunderstood this plugin:
> > https://github.com/jenkinsci/ghprb-plugin
>
>
>
> Maybe you are right, I am not using that plugin on the Jenkins of my
> company.
> In Bookkeeper actually we are using ${ghprbActualCommit} and not ${sha1}.
>
Oh I see. I think I changed it to ${ghprbActualCommit} to support PRs that
run against branches.
>
> I got this idea because you (Sijie) were asking Jia to rebase a PR to
> current master in order to suppress a flaky test.
>
Good point. That's probably due to ${ghprbActualCommit}. ${sha1} probably
would address the problem.
It would be good to test if ${sha1} works a) does what you want b) can work
with branches.
> Using that "Additional behavior" of Jenkins GIT support I never had such
> problem, just re-kicking the job applies current master and picks up all
> the improvements.
>
Agreed. fast-forward merge should be helpful. I think it is no harm to
enable that.
> It is also safer to have the job run with the branch merged to current
> master, because the effect is the same as what the committer would have
> done by running tests/QA checks while using the merge script (And I never
> do it during the script because it takes too much time).
>
I think tests/QA checks can be removed from merge script since we are
blocking merges if CI doesn't pass.
>
>
> Enrico
>
>
>
> >
> >
> > On Mon, Jul 23, 2018 at 11:41 PM Enrico Olivelli <eo...@gmail.com>
> > wrote:
> >
> > > Hi,
> > > I am surprised that in our pre-commit jobs on Jenkins we are not
> merging
> > > the Pull Request Branch with current master before running all
> > > tests/checks.
> > >
> > > Ideally we have to simulate the final status of the repository after
> > > merging the branch to "master"
> > >
> > >
> > > This can be simply achieved but adding an "Additional behaviour" on GIT
> > > configuration:
> > >
> > > Additional Behaviours - Merge Before build:
> > > Name of repository: origin
> > > Branch to merge to: master
> > > Merge strategy: default
> > > Fast forward mode: --ff
> > >
> > > This is the configuration which I am using for all my projects and it
> > works
> > > like a charm
> > >
> > > Am I missing something ?
> > > If you agree I will submit a patch to change all the jobs and add this
> > > magic trick
> > >
> > > Enrico
> > >
> >
>
Re: Precommit Jobs - don't we merge to master before tests ?
Posted by Enrico Olivelli <eo...@gmail.com>.
Il giorno mar 24 lug 2018 alle ore 09:25 Sijie Guo <gu...@gmail.com> ha
scritto:
> I am not sure need this configuration.
>
> based on my understanding, if we are using `${sha1}`, jenkins will use
> Github's tentative merge of the compare and base branches (e.g.
> refs/pull/<pull-request-id>/merge) if the PR can be automatically merged or
> the head of the pull request (e.g. refs/pull/<pull-request-id>/head) if
> they can not be automatically merged.
>
> since we only merge a PR that all conflicts are resolved, that means the
> final precommit check status are on the tentative merge of the compare and
> base branches.
>
> Unless I misunderstood this plugin:
> https://github.com/jenkinsci/ghprb-plugin
Maybe you are right, I am not using that plugin on the Jenkins of my
company.
In Bookkeeper actually we are using ${ghprbActualCommit} and not ${sha1}.
I got this idea because you (Sijie) were asking Jia to rebase a PR to
current master in order to suppress a flaky test.
Using that "Additional behavior" of Jenkins GIT support I never had such
problem, just re-kicking the job applies current master and picks up all
the improvements.
It is also safer to have the job run with the branch merged to current
master, because the effect is the same as what the committer would have
done by running tests/QA checks while using the merge script (And I never
do it during the script because it takes too much time).
Enrico
>
>
> On Mon, Jul 23, 2018 at 11:41 PM Enrico Olivelli <eo...@gmail.com>
> wrote:
>
> > Hi,
> > I am surprised that in our pre-commit jobs on Jenkins we are not merging
> > the Pull Request Branch with current master before running all
> > tests/checks.
> >
> > Ideally we have to simulate the final status of the repository after
> > merging the branch to "master"
> >
> >
> > This can be simply achieved but adding an "Additional behaviour" on GIT
> > configuration:
> >
> > Additional Behaviours - Merge Before build:
> > Name of repository: origin
> > Branch to merge to: master
> > Merge strategy: default
> > Fast forward mode: --ff
> >
> > This is the configuration which I am using for all my projects and it
> works
> > like a charm
> >
> > Am I missing something ?
> > If you agree I will submit a patch to change all the jobs and add this
> > magic trick
> >
> > Enrico
> >
>
Re: Precommit Jobs - don't we merge to master before tests ?
Posted by Sijie Guo <gu...@gmail.com>.
I am not sure need this configuration.
based on my understanding, if we are using `${sha1}`, jenkins will use
Github's tentative merge of the compare and base branches (e.g.
refs/pull/<pull-request-id>/merge) if the PR can be automatically merged or
the head of the pull request (e.g. refs/pull/<pull-request-id>/head) if
they can not be automatically merged.
since we only merge a PR that all conflicts are resolved, that means the
final precommit check status are on the tentative merge of the compare and
base branches.
Unless I misunderstood this plugin:
https://github.com/jenkinsci/ghprb-plugin
On Mon, Jul 23, 2018 at 11:41 PM Enrico Olivelli <eo...@gmail.com>
wrote:
> Hi,
> I am surprised that in our pre-commit jobs on Jenkins we are not merging
> the Pull Request Branch with current master before running all
> tests/checks.
>
> Ideally we have to simulate the final status of the repository after
> merging the branch to "master"
>
>
> This can be simply achieved but adding an "Additional behaviour" on GIT
> configuration:
>
> Additional Behaviours - Merge Before build:
> Name of repository: origin
> Branch to merge to: master
> Merge strategy: default
> Fast forward mode: --ff
>
> This is the configuration which I am using for all my projects and it works
> like a charm
>
> Am I missing something ?
> If you agree I will submit a patch to change all the jobs and add this
> magic trick
>
> Enrico
>