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:41:01 GMT
Pavel,

> I think this thread is a good opportunity to discuss commit message guidelines.

We had a thread about it recently [1].

[1] https://lists.apache.org/thread.html/rde6e8258537704433286a60e1d0142485c25228a46561544d35b9704%40%3Cdev.ignite.apache.org%3E

Best regards,
Ivan Pavlukhin

пн, 4 мая 2020 г. в 10:38, Ivan Pavlukhin <vololo100@gmail.com>:
>
> 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