ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Pavlukhin <vololo...@gmail.com>
Subject Re: [DISCUSSION] Pull-request checks on GitHub
Date Mon, 04 May 2020 07:38:03 GMT
Maxim,

Thanks again for doing great things!

Out of curiosity, could you please shed a light why there are 2 travis
checks for PR [1]? I am about checks named
continuous-integration/travis-ci/pr and
continuous-integration/travis-ci/push.

Best regards,
Ivan Pavlukhin

вс, 3 мая 2020 г. в 13:08, Pavel Tupitsyn <ptupitsyn@apache.org>:
>
> Igniters, Maxim,
>
> I think this thread is a good opportunity to discuss commit message
> guidelines.
> I suggest the following:
>
> 1. Treat PR title + description as the final squashed commit message.
> PR author is responsible for writing that properly.
> Committer who merges the PR is responsible for validating that and using
> that for the actual squash commit.
>
> 2. Adopt the following Git commit message rules (partially from
> https://chris.beams.io/posts/git-commit/):
> - Start with IGNITE-NNNN
> - Use imperative mood in the subject line ("Fix foobar crash on start",
> "Add baz metric")
> - Capitalize the subject line
> - Do not end the subject line with a period
> - Use the body to explain what and why vs. how
>
> Thoughts?
>
> On Sun, May 3, 2020 at 11:53 AM Maxim Muzafarov <mmuzaf@apache.org> wrote:
>
> > Hello,
> >
> > I have the following in my mind:
> > 1. This checklist is for discussion and may be changed.
> > 2. Commits can be squashed in the branch prior to asking a review, but
> > when the review is in progress a good naming may help to understand
> > the changes.
> > 3. It's true that the commit message can be changed prior to merging
> > the master branch, but it's better to merge the PR with an initial
> > authored commit message `as is`.
> >
> > On Sat, 2 May 2020 at 18:20, Guru Stron <gurustronpublic@gmail.com> wrote:
> > >
> > > Maxim,
> > >
> > > I have a small question about "Commits have the following pattern..". Is
> > > it really needed cause AFAIK commits in the PR are squashed. Or am  I
> > > missing something?
> > >
> > > On Thu, Apr 30, 2020, 8:33 PM Maxim Muzafarov <mmuzaf@apache.org> wrote:
> > >
> > > > Folks,
> > > >
> > > >
> > > > I've created the pull request template for GitHub.
> > > > Please, take a look and let me know what you think [1] [2].
> > > >
> > > >
> > > > [1]
> > > >
> > https://github.com/apache/ignite/pull/7765/files#diff-195a635ad245ded9076330e31134bd80
> > > > [2] https://issues.apache.org/jira/browse/IGNITE-12937
> > > >
> > > > On Sun, 26 Apr 2020 at 20:35, Saikat Maitra <saikat.maitra@gmail.com>
> > > > wrote:
> > > > >
> > > > > Hi Maxim,
> > > > >
> > > > > Thank you for enabling travis ci in ignite repo. It is very helpful
> > to
> > > > see
> > > > > PR build results integrated in PR request.
> > > > >
> > > > > I will enable it in ignite-extensions repo as well.
> > > > >
> > > > > Regards,
> > > > > Saikat
> > > > >
> > > > > On Mon, Apr 20, 2020 at 12:14 PM Pavel Tupitsyn <
> > ptupitsyn@apache.org>
> > > > > wrote:
> > > > >
> > > > > > Maxim, pull request template is a great idea.
> > > > > > We can put a checklist there along with the links to the
> > guidelines,
> > > > > > something like this:
> > > > > >
> > > > > > [ ] Coding Guidelines are followed
> > > > > > [ ] TeamCity build passes
> > > > > > [ ] JIRA ticked is in Patch Available state, review has been
> > requested
> > > > in
> > > > > > comments
> > > > > > [ ] Something else?
> > > > > >
> > > > > > On Mon, Apr 20, 2020 at 8:09 PM Maxim Muzafarov <mmuzaf@apache.org
> > >
> > > > wrote:
> > > > > >
> > > > > > > Pavel,
> > > > > > >
> > > > > > > Sorry for the incomplete message.
> > > > > > >
> > > > > > > 2. I mentioned it too. Hopefully, builds > 4 hrs would
not be too
> > > > often.
> > > > > > > The reason - there are limited job-workers shared between
all the
> > > > > > > Apache projects. I found some details here [1] [2].
> > > > > > >
> > > > > > >
> > > > > > > [1]
> > > > > > >
> > > > > >
> > > >
> > https://lists.apache.org/thread.html/af52e2a3e865c01596d46374e8b294f2740587dbd59d85e132429b6c@%3Cbuilds.apache.org%3E
> > > > > > > [2] https://issues.apache.org/jira/browse/INFRA-18533
> > > > > > >
> > > > > > > On Mon, 20 Apr 2020 at 20:03, Maxim Muzafarov <mmuzaf@apache.org
> > >
> > > > wrote:
> > > > > > > >
> > > > > > > > Pavel,
> > > > > > > >
> > > > > > > > 1. Agree here. What if we create a default template
pull
> > request
> > > > > > > > description with all the links required by our development
> > process?
> > > > > > > > [1] It's will be friendly for contributors to have
everything
> > they
> > > > > > > > need in the PR.
> > > > > > > >
> > > > > > > > 2.
> > > > > > > >
> > > > > > > > [1]
> > > > > > >
> > > > > >
> > > >
> > https://help.github.com/en/github/building-a-strong-community/creating-a-pull-request-template-for-your-repository
> > > > > > > >
> > > > > > > > On Mon, 20 Apr 2020 at 19:46, Pavel Tupitsyn <
> > ptupitsyn@apache.org
> > > > >
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Maxim,
> > > > > > > > >
> > > > > > > > > Good news, thank you.
> > > > > > > > >
> > > > > > > > > However, I see two issues with this:
> > > > > > > > >
> > > > > > > > > 1. False sense of a ready-to-merge PR
> > > > > > > > > Now that we have a reassuring green checkmark
on the PR,
> > > > contributors
> > > > > > > might
> > > > > > > > > think that build passes and all is well.
> > > > > > > > > But this is not true - we only check that the
code compiles.
> > > > TeamCity
> > > > > > > run
> > > > > > > > > is still required.
> > > > > > > > > My proposal is to change the text somehow to
make this clear,
> > > > maybe
> > > > > > > add a
> > > > > > > > > link to the contribution guidelines automatically.
> > > > > > > > >
> > > > > > > > > 2. Builds seem to spend a lot of time in the
queue.
> > > > > > > > > I've created this PR 4 hours ago, still no results:
[1]
> > > > > > > > > Any ideas? I use Travis on some other GitHub
projects and it
> > > > usually
> > > > > > > runs
> > > > > > > > > in a minute or two.
> > > > > > > > >
> > > > > > > > > [1] https://github.com/apache/ignite/pull/7698
> > > > > > > > >
> > > > > > > > > On Mon, Apr 20, 2020 at 3:16 PM Maxim Muzafarov
<
> > > > mmuzaf@apache.org>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Igniters,
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > The Travis-ci build configured for running
on the Apache
> > > > Ignite PRs
> > > > > > > > > > and the master branch [1] [2].
> > > > > > > > > >
> > > > > > > > > > Build run under:
> > > > > > > > > > openjdk8
> > > > > > > > > > openjdk11
> > > > > > > > > >
> > > > > > > > > > Example of PR:
> > > > > > > > > > https://github.com/apache/ignite/pull/7695
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > [1] https://travis-ci.org/github/apache/ignite
> > > > > > > > > > [2] https://issues.apache.org/jira/browse/IGNITE-12916
> > > > > > > > > >
> > > > > > > > > > On Tue, 14 Apr 2020 at 21:00, Maxim Muzafarov
<
> > > > mmuzaf@apache.org>
> > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Petr,
> > > > > > > > > > >
> > > > > > > > > > > I think it's doable. It has custom
`install-jdk` script,
> > so
> > > > even
> > > > > > > the
> > > > > > > > > > > latest JDKs can be used.
> > > > > > > > > > >
> > > > > > > > > > > [1] https://github.com/sormuras/bach#install-jdksh
> > > > > > > > > > >
> > > > > > > > > > > On Tue, 14 Apr 2020 at 18:31, Petr
Ivanov <
> > > > mr.weider@gmail.com>
> > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > We do not need JDK10 — it is
out of support already.
> > > > > > > > > > > > Instead, how about adding JDK14?
> > > > > > > > > > > >
> > > > > > > > > > > > > On 14 Apr 2020, at 17:30,
Maxim Muzafarov <
> > > > mmuzaf@apache.org
> > > > > > >
> > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Folks,
> > > > > > > > > > > > >
> > > > > > > > > > > > > I forgot to mention one more
important thing of this
> > > > tool. We
> > > > > > > can
> > > > > > > > > > > > > configure build and checks
simultaneously for
> > several JDK
> > > > > > > versions.
> > > > > > > > > > > > >
> > > > > > > > > > > > > jdk:
> > > > > > > > > > > > >  - oraclejdk8
> > > > > > > > > > > > >  - openjdk10
> > > > > > > > > > > > >  - openjdk11
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Tue, 14 Apr 2020 at 17:17,
Maxim Muzafarov <
> > > > > > > mmuzaf@apache.org>
> > > > > > > > > > wrote:
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Folks,
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> +1 Travis-ci
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> I see no disadvantages
of having multiple CI tools
> > due
> > > > to:
> > > > > > > > > > > > >> - it's free for open-source
and can be disabled at
> > any
> > > > time
> > > > > > > without
> > > > > > > > > > > > >> any consequences;
> > > > > > > > > > > > >> - it will free TeamCity
from running builds on each
> > PR
> > > > and
> > > > > > TC
> > > > > > > can
> > > > > > > > > > > > >> focus on tests execution;
> > > > > > > > > > > > >> - we can perform more
sophisticated checks with this
> > > > tool
> > > > > > > like a PR
> > > > > > > > > > > > >> title format (e.g. IGNITE-XXXXX:
Sample)
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> It seems the TC.Bot can
also be integrated with
> > GitHub
> > > > > > checks
> > > > > > > via
> > > > > > > > > > REST API [1].
> > > > > > > > > > > > >>
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> I've checked locally
the Ignite build procedure with
> > > > > > > travis-ci and
> > > > > > > > > > > > >> GitHub checks [2] and
looks like everything is
> > working
> > > > fine.
> > > > > > > > > > > > >> Who can configure the
similar things on Apache
> > Ignite
> > > > GitHub
> > > > > > > mirror?
> > > > > > > > > > > > >> Does anyone have such
access rights?
> > > > > > > > > > > > >>
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> [1] https://developer.github.com/v3/checks/runs/
> > > > > > > > > > > > >> [2]
> > > > > > > > > >
> > > > > > >
> > > > https://github.com/Mmuzaf/ignite/pull/1/checks?check_run_id=584537955
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> On Tue, 14 Apr 2020 at
10:37, Nikolay Izhikov <
> > > > > > > nizhikov@apache.org>
> > > > > > > > > > wrote:
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>>> On another hand,
it seems weird to have both
> > TeamCity
> > > > and
> > > > > > > Travis
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>> And don’t forget
MTCGA bot!
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>>> 14 апр. 2020
г., в 10:23, Pavel Tupitsyn <
> > > > > > > ptupitsyn@apache.org>
> > > > > > > > > > написал(а):
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> We should have
PR checks for sure.
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> On one hand,
I agree with Denis:
> > > > > > > > > > > > >>>> - Travis is very
easy to set up in GitHub
> > > > > > > > > > > > >>>> - Config file
(travis.yml) is stored in git,
> > which is
> > > > > > great
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> On another hand,
it seems weird to have both
> > TeamCity
> > > > and
> > > > > > > Travis.
> > > > > > > > > > > > >>>> Thoughts?
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> On Tue, Apr 14,
2020 at 10:16 AM Denis Garus <
> > > > > > > garus.d.g@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>>> Hello!
> > > > > > > > > > > > >>>>>
> > > > > > > > > > > > >>>>> I have seen
projects with Travis-ci they look
> > cool.
> > > > > > > > > > > > >>>>> I think Travis-ci
is a good solution.
> > > > > > > > > > > > >>>>>
> > > > > > > > > > > > >>>>> вт, 14
апр. 2020 г. в 10:00, Andrey Mashenkov <
> > > > > > > > > > andrey.mashenkov@gmail.com
> > > > > > > > > > > > >>>>>> :
> > > > > > > > > > > > >>>>>
> > > > > > > > > > > > >>>>>> Maxim,
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>> Good
idea. I'd add a license check as well.
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>> On Tue,
Apr 14, 2020 at 2:14 AM Maxim Muzafarov
> > <
> > > > > > > > > > mmuzaf@apache.org>
> > > > > > > > > > > > >>>>> wrote:
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>>> Igniters,
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>> It's
really `must-have` feature for me to
> > enable
> > > > Apache
> > > > > > > Ignite
> > > > > > > > > > > > >>>>>>> pull-request
status checks on GitHub.
> > Currently we
> > > > > > don't
> > > > > > > have
> > > > > > > > > > any of
> > > > > > > > > > > > >>>>>>> them.
The most obvious check for each
> > pull-request
> > > > is:
> > > > > > > > > > > > >>>>>>> -
build the source code and check code-style
> > > > > > violations;
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>> This
will give us some advantages. For
> > instance:
> > > > > > > > > > > > >>>>>>> 1.
Each PR even a very simple (not require
> > tests
> > > > > > > execution)
> > > > > > > > > > will be
> > > > > > > > > > > > >>>>>>> checked
by checkstyle and for compile errors.
> > > > > > > > > > > > >>>>>>> 2.
Developers can be get notified earlier if
> > > > checkstyle
> > > > > > > has
> > > > > > > > > > been
> > > > > > > > > > > > >>>>>>> violated
in their PRs.
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>> To
achieve this we can do the following:
> > > > > > > > > > > > >>>>>>> 1.
Configure our TeamCity integration with the
> > > > Ignite
> > > > > > > GitHub
> > > > > > > > > > > > >>>>>>> repository
and PR build. It seems it is
> > possible
> > > > [2].
> > > > > > > > > > > > >>>>>>> 2.
Use Travis-ci which is free for open-source
> > > > projects
> > > > > > > and
> > > > > > > > > > also has
> > > > > > > > > > > > >>>>>>> an
integration with GitHub checks [1].
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>> What
do you think?
> > > > > > > > > > > > >>>>>>> What
options will be the best for us?
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>> [1]
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>
> > > > > > > > > >
> > > > > > >
> > > > > >
> > > >
> > https://blog.travis-ci.com/2018-05-07-announcing-support-for-github-checks-api-on-travis-ci-com
> > > > > > > > > > > > >>>>>>> [2]
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>
> > > > > > > > > >
> > > > > > >
> > > > > >
> > > >
> > https://himynameistim.com/2018/01/16/adding-build-statuses-to-pull-requests-with-teamcity-and-github/
> > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>> --
> > > > > > > > > > > > >>>>>> Best
regards,
> > > > > > > > > > > > >>>>>> Andrey
V. Mashenkov
> > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > >>>>>
> > > > > > > > > > > > >>>
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > >
> > > >
> > > Y
> > >
> > > >
> >

Mime
View raw message