ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Artem Shutak <ashu...@gridgain.com>
Subject Re: IGNITE-1217 TC triggering by github pull-requests
Date Wed, 26 Aug 2015 18:06:34 GMT
Folks,

I've implemented a script for applying pull-requests by commiters and
updated info about pull-request contribution way. I think, it should be
clear for everyone now. Feel free to comment:
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute.

I didn't know about github feature with closing pull-request by special
comment. I will implement it shortly, but I've described on wiki that it's
already implemented (I think it's not an issue).

-- Artem --

On Sat, Aug 15, 2015 at 4:05 AM, Konstantin Boudnik <cos@apache.org> wrote:

> On Fri, Aug 14, 2015 at 03:23PM, Valentin Kulichenko wrote:
> > Artem,
> >
> > Why do we need a requirement about one commit per pull request? I
> > understand that we should should avoid garbage in history and properly
> deal
> > with merges from master. But I don't see anything wrong with 2-3 commits
> in
> > a PR if they are meaningful. E.g., "implemented initial version" ->
> "fixed
> > failures in tests" -> "addressed comments from a committer". They should
> > just contain good comments and (probably) corresponding JIRA ticket
> number.
>
> Because such incremental commits for the same JIRA have two properties:
>  - they are largely irrelevant and lack of substance for anyone but their
> author
>  - they make later navigation through the history a royal PITA because you
>    never know if this JIRA had one commit or six
>
> Cos
>
> > In your process (if I understood everything correctly) we will create two
> > PRs for each feature. This looks weird and confusing for me. And I really
> > don't understand what issue we're trying to solve here.
> >
> > I think we should take a look at Spark .py script that can be found here:
> >
> https://cwiki.apache.org/confluence/display/SPARK/Committers#Committers-HowtoMergeaPullRequest
> .
> > It looks like they already addressed and automated everything we discuss
> > here.
> >
> > Thoughts?
> >
> > -Val
> >
> > On Fri, Aug 14, 2015 at 7:03 AM, Denis Magda <dmagda@gridgain.com>
> wrote:
> >
> > > Artem,
> > >
> > > Seems that TC doesn't write down comments into a corresponding JIRA
> issue
> > > for branches that have 'dev' suffix (like 'ignite-xxx-dev').
> > >
> > > I've created a pull-request to merge from 'ignite-1241-dev':
> > > https://github.com/apache/incubator-ignite/pull/19
> > >
> > > I've received an e-mail saying that the request has been received and I
> > > see that TC triggered the request for validation.
> > > However, there is no any info left regarding this in a corresponding
> > > ticket:
> > > https://issues.apache.org/jira/browse/IGNITE-1241
> > >
> > > Meanwhile, here I see that such info from GitHub Bot:
> > > https://issues.apache.org/jira/browse/IGNITE-1203
> > >
> > > In addition I have one more thing to clarify.
> > > When TC ends validating my changes will it send links to tests' results
> > > over email or to JIRA ticket?
> > > Cause as you know TC cleans 'active branches' history and I guess that
> it
> > > won't be easy for me to find the results on TC in a couple of days.
> > >
> > > --
> > > Denis
> > >
> > >
> > > On 8/14/2015 1:30 PM, Artem Shutak wrote:
> > >
> > >> Alexey,
> > >>
> > >> You are right and big thanks for the diagram on the wiki!
> > >>
> > >> In bounds of IGNITE-1217
> > >> <https://issues.apache.org/jira/browse/IGNITE-1217> I've
> > >> created a script for commiters (see patch). You need to point to the
> > >> script
> > >> a contributor_github_user_name and a branch_name_with_contribution.
> The
> > >> script just fetchs a remote repository (by
> contributor_github_user_name)
> > >> and cherry-picks last commit from this branch to local master - it
> stores
> > >> commit author information. So, the commiter can push master at apache
> git.
> > >>
> > >> In this case we have one requirement: a pull-request should have only
> one
> > >> commit (as with patches).
> > >>
> > >> I've mention next model of development at fork (see steps below). But
> now,
> > >> I see it has too many steps. I will think about more simple way.
> > >> 1. Preparation (need to configure once):
> > >>
> > >> git remote add apache
> > >> https://git-wip-us.apache.org/repos/asf/incubator-ignite
> > >> git remote my_fork https://github.com/
> <github_uname>/incubator-ignite.git
> > >>
> > >> 2. Forking on jira ignite-xxxx
> > >> # Update local master from apache repo
> > >> git checkout master
> > >> git pull apache master
> > >>
> > >> # Create new development branch for the ticket.
> > >> git checkout -b ignite-xxxx-dev
> > >>
> > >> # Some development here with many commits at ignite-xxxx-dev.
> > >> git commit -a -m 'ignite-xxxx: Intermediate commit 1'
> > >> ...
> > >> git commit -a -m 'ignite-xxxx: Intermediate commit 100'
> > >>
> > >> # Push at fork
> > >> git push my_fork ignite-xxxx-dev
> > >>
> > >> Now a pull-request can be created. TC will be triggered. To fix some
> > >> something and rerun TC, you need just fix it at ignite-xxxx-dev and
> push
> > >> to
> > >> fork again, TC will be triggered again for new commit.
> > >>
> > >> When TC is green, it needs to create 'final' pull-request branch with
> one
> > >> commit. For it:
> > >> # Update local master from apache repo
> > >> git checkout master
> > >> git pull apache master
> > >>
> > >> # Create a final pull-request branch for the ticket.
> > >> git checkout -b ignite-xxxx
> > >>
> > >> # Squash all changes at one commit.
> > >> git merge --squash ignite-xxxx-dev
> > >> git commit -a -m 'ignite-xxxx: Implemented'
> > >>
> > >> Then need to push it to fork and open new pull-request.
> > >>
> > >>
> > >> -- Artem --
> > >>
> > >> On Fri, Aug 14, 2015 at 5:30 AM, Alexey Goncharuk <
> > >> alexey.goncharuk@gmail.com> wrote:
> > >>
> > >> I forgot that the mailing list takes out all formatting, the diagram
> meant
> > >>> to be in a monospaced font :)
> > >>>
> > >>> I added it to Ignite wiki:
> > >>> https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute
> > >>>
> > >>> 2015-08-13 19:22 GMT-07:00 Alexey Goncharuk <
> alexey.goncharuk@gmail.com
> > >>> >:
> > >>>
> > >>> While the process of a pull-request creation and CI run is clear
> for, the
> > >>>> whole cycle from start to end is still fuzzy. Let me summarize
my
> > >>>> understanding and correct me if I got something wrong.
> > >>>>
> > >>>> For any committer/contributor John Doe we will have the following
> > >>>> structure:
> > >>>>
> > >>>>   +------------+             +---------------+
> > >>>>   +-----------------+
> > >>>>   |            |   replica   |               |    fork    |
> > >>>> |
> > >>>>   | Apache Git | ==========> | GitHub Mirror | --------->
| John
> Doe's
> > >>>>
> > >>> Fork
> > >>>
> > >>>> |
> > >>>>   |            |             |               |            |
> > >>>> |
> > >>>>   +------------+             +---------------+
> > >>>>   +-----------------+
> > >>>>          ^                                                    
    ^
> > >>>>          |                                                    
    |
> > >>>>          |                                                    
    |
> > >>>>          |
> > >>>>   +-----------------+
> > >>>>          |    *Apache Git remote handle for committers*   |
> > >>>> |
> > >>>>          +------------------------------------------------|   Local
> > >>>> clone
> > >>>> |
> > >>>>                                                           |
> > >>>> |
> > >>>>
> > >>>>   +-----------------+
> > >>>> Development is going in the JD's fork and at some point he thinks
> that
> > >>>>
> > >>> the
> > >>>
> > >>>> feature is ready to be tested by CI.
> > >>>>
> > >>>> He creates a pull request. Usually it takes more than one iteration
> to
> > >>>> have a successful CI run, but each pull request sends an e-mail
to
> the
> > >>>>
> > >>> dev
> > >>>
> > >>>> list. I think we should have some mechanism allowing to
> differentiate
> > >>>> "work" pull requests and final pull requests that passed CI and
> should
> > >>>> be
> > >>>> reviewed by a committer. We also need to create (maybe) a maven
> profile
> > >>>> with a set of quick tests that cover as much functionality as
> possible,
> > >>>>
> > >>> so
> > >>>
> > >>>> that a developer could run it locally before submitting a request
> to the
> > >>>>
> > >>> CI.
> > >>>
> > >>>> Let's say now the pull request is approved. If the pull request
was
> > >>>> submitted by a contributor, a committer should pull it to it's
local
> > >>>>
> > >>> clone.
> > >>>
> > >>>> Then commit is pushed to the apache git repository. I glanced
> through
> > >>>> the
> > >>>> Apache Spark development process document [1] and it seems that
we
> > >>>> should
> > >>>> have a similar script that will properly process commits (squash
or
> > >>>> whatever we need) before the push.
> > >>>>
> > >>>> Assuming my understanding is correct and the minor things I
> mentioned
> > >>>> above are addressed, I like the new process :)
> > >>>>
> > >>>> [1]
> > >>>>
> https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark
> > >>>>
> > >>>>
> > >>>> 2015-08-13 18:13 GMT-07:00 Konstantin Boudnik <cos@apache.org>:
> > >>>>
> > >>>> On Thu, Aug 13, 2015 at 05:54PM, Dmitriy Setrakyan wrote:
> > >>>>>
> > >>>>>> On Thu, Aug 13, 2015 at 5:51 PM, Konstantin Boudnik <
> cos@apache.org>
> > >>>>>>
> > >>>>> wrote:
> > >>>>>
> > >>>>>> On Thu, Aug 13, 2015 at 05:40PM, Alexey Goncharuk wrote:
> > >>>>>>>
> > >>>>>>>> Maybe I miss a good piece of information about
how Git works,
> but
> > >>>>>>>>
> > >>>>>>> I
> > >>>
> > >>>> always
> > >>>>>>>
> > >>>>>>>> thought that if a pull request is accepted, it
will be merged to
> > >>>>>>>>
> > >>>>>>> the
> > >>>
> > >>>> GitHub
> > >>>>>>>
> > >>>>>>>> mirror of Apache Ignite. How will this change get
to the
> original
> > >>>>>>>>
> > >>>>>>> Apache
> > >>>>>
> > >>>>>> git repository?
> > >>>>>>>>
> > >>>>>>> It won't. github repo is a mirror of Apache git mirror.
In order
> to
> > >>>>>>>
> > >>>>>> have
> > >>>>>
> > >>>>>> the
> > >>>>>>> changes from github PR to be in visible in the github
a committer
> > >>>>>>>
> > >>>>>> needs to
> > >>>>>
> > >>>>>> commit it into our Apache repo.
> > >>>>>>>
> > >>>>>>> Cos, will the original contributor's name be preserved
or should
> the
> > >>>>>>
> > >>>>> Ignite
> > >>>>>
> > >>>>>> committer add "-author" parameter when committing?
> > >>>>>>
> > >>>>> It depends on how the patch file was made. If 'git format-patch'
> was
> > >>>>>
> > >>>> used
> > >>>
> > >>>> then
> > >>>>> the name will be preserved. Otherwise, it won't. Sorry, I don't
> know
> > >>>>>
> > >>>> much
> > >>>
> > >>>> about github and I really am not using it.
> > >>>>>
> > >>>>> Cos
> > >>>>>
> > >>>>> Can somebody explain to me the merge procedure?
> > >>>>>>>>
> > >>>>>>>> 2015-08-12 3:15 GMT-07:00 Artem Shutak <ashutak@gridgain.com>:
> > >>>>>>>>
> > >>>>>>>> Inline.
> > >>>>>>>>>
> > >>>>>>>>> On Wed, Aug 12, 2015 at 10:19 AM, Dmitriy Setrakyan
<
> > >>>>>>>>>
> > >>>>>>>> dsetrakyan@apache.org
> > >>>>>>>
> > >>>>>>>> wrote:
> > >>>>>>>>>
> > >>>>>>>>> On Tue, Aug 11, 2015 at 6:28 AM, Artem Shutak
<
> > >>>>>>>>>>
> > >>>>>>>>> ashutak@gridgain.com>
> > >>>>>
> > >>>>>> wrote:
> > >>>>>>>>>>
> > >>>>>>>>>> And one more question. Is it mandatory
to have possibility
> > >>>>>>>>>>>
> > >>>>>>>>>> works
> > >>>>>
> > >>>>>> via
> > >>>>>>>
> > >>>>>>>> patches if we will have pull-request way for contributions?
> > >>>>>>>>>>>
> > >>>>>>>>>>> I'd like to have only one approach.
> > >>>>>>>>>>>
> > >>>>>>>>>>> Artem, if possible I would allow 2
approaches and document
> > >>>>>>>>>>
> > >>>>>>>>> the 2
> > >>>
> > >>>> approaches
> > >>>>>>>>>
> > >>>>>>>>>> on Wiki.
> > >>>>>>>>>>
> > >>>>>>>>>> At least it increases support efforts.
And if all will use
> only
> > >>>>>>>>>
> > >>>>>>>> one,
> > >>>>>
> > >>>>>> then
> > >>>>>>>
> > >>>>>>>> there is a big chance that second will not work
properly.
> > >>>>>>>>>
> > >>>>>>>>> And, to complete patch-way:
> > >>>>>>>>> - need to split simple "master" builds and
"patch" builds on TC
> > >>>>>>>>>
> > >>>>>>>> -
> > >>>
> > >>>> I
> > >>>>>
> > >>>>>> can do
> > >>>>>>>
> > >>>>>>>> it by yourself.
> > >>>>>>>>> - need to implement git-format-patch.bat for
Windows users.
> It's
> > >>>>>>>>>
> > >>>>>>>> not
> > >>>>>
> > >>>>>> mandatory, all can be done manually by contributors, but
it
> > >>>>>>>>>
> > >>>>>>>> would
> > >>>
> > >>>> be
> > >>>>>
> > >>>>>> nice.
> > >>>>>>>
> > >>>>>>>> This script can make any Windows user (I'm not
:) ).
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>> One question, does a pull request automatically
generate a
> > >>>>>>>>>>
> > >>>>>>>>> Jira
> > >>>
> > >>>> comment
> > >>>>>>>
> > >>>>>>>> (see Spark, Camel)?
> > >>>>>>>>>>
> > >>>>>>>>>> I will look at mentioned projects. From
my view, by default,
> > >>>>>>>>>
> > >>>>>>>> GitHub
> > >>>>>
> > >>>>>> know
> > >>>>>>>
> > >>>>>>>> nothing about our Jira. So, there's no way to GitHub
can add any
> > >>>>>>>>>
> > >>>>>>>> comments
> > >>>>>>>
> > >>>>>>>> to unknown Jira.
> > >>>>>>>>> DVCS plugin - it's a standard way to acquaint
Jira and GitHub
> > >>>>>>>>>
> > >>>>>>>> and
> > >>>
> > >>>> it
> > >>>>>
> > >>>>>> will
> > >>>>>>>
> > >>>>>>>> work pretty nice.
> > >>>>>>>>>
> > >>>>>>>>> --Artem--
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>>> -- Artem --
> > >>>>>>>>>>>
> > >>>>>>>>>>> On Tue, Aug 11, 2015 at 4:15 PM, Artem
Shutak <
> > >>>>>>>>>>>
> > >>>>>>>>>> ashutak@gridgain.com>
> > >>>>>>>
> > >>>>>>>> wrote:
> > >>>>>>>>>>>
> > >>>>>>>>>>> Igniters,
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> I'm working on
> > >>>>>>>>>>>>
> > >>>>>>>>>>> https://issues.apache.org/jira/browse/IGNITE-1217
> > >>>>>
> > >>>>>> .
> > >>>>>>>
> > >>>>>>>> Currently, everyone can fork Mirror of Apache Ignite
on
> > >>>>>>>>>>>>
> > >>>>>>>>>>> GitHub (
> > >>>>>
> > >>>>>> https://github.com/apache/incubator-ignite), works with
> > >>>>>>>>>>>>
> > >>>>>>>>>>> own fork
> > >>>>>
> > >>>>>> (create
> > >>>>>>>>>>
> > >>>>>>>>>>> branches, commit, pull changes at fork)
and then creates a
> > >>>>>>>>>>>>
> > >>>>>>>>>>> pull-request
> > >>>>>>>>>
> > >>>>>>>>>> to
> > >>>>>>>>>>>
> > >>>>>>>>>>>> Mirror of Apache Ignite on GitHub
(all changes should be
> > >>>>>>>>>>>>
> > >>>>>>>>>>> done in
> > >>>>>
> > >>>>>> one
> > >>>>>>>
> > >>>>>>>> commit
> > >>>>>>>>>>>
> > >>>>>>>>>>>> as in patch-way approach). Then
test TC builds will
> > >>>>>>>>>>>>
> > >>>>>>>>>>> triggered
> > >>>>>
> > >>>>>> automatically. Results can be found by branch filtering
by
> > >>>>>>>>>>>>
> > >>>>>>>>>>> pattern
> > >>>>>>>
> > >>>>>>>> <pull-request-number>/merge. "Merge" suffix
here means
> > >>>>>>>>>>>>
> > >>>>>>>>>>> pull-request
> > >>>>>>>
> > >>>>>>>> merged
> > >>>>>>>>>>>
> > >>>>>>>>>>>> with master branch (if pull-request
at master branch).
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Notes:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> 1. I tried to use TC plugin for
github to see TC result at
> > >>>>>>>>>>>>
> > >>>>>>>>>>> pull-request.
> > >>>>>>>>>>
> > >>>>>>>>>>> But the plugin works in unexpected
way and add comments
> > >>>>>>>>>>>>
> > >>>>>>>>>>> not
> > >>>
> > >>>> only
> > >>>>>
> > >>>>>> to
> > >>>>>>>
> > >>>>>>>> pull-requests. Example:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>
> https://github.com/apache/incubator-ignite/commit/ae11e9b5aa9af4d0d58e2a16dd3a3331969961df#commitcomment-12635375
> > >>>
> > >>>> .
> > >>>>>>>>>>>> Maybe someone had this problem
before?
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> 2. I'm looking for a simple way
to add information about
> > >>>>>>>>>>>>
> > >>>>>>>>>>> new
> > >>>
> > >>>> pull-request
> > >>>>>>>>>>
> > >>>>>>>>>>> to associated jira.
> > >>>>>>>>>>>> The better way to use existing
Jira plugin for it - DVCS
> > >>>>>>>>>>>>
> > >>>>>>>>>>> plugin (
> > >>>>>
> > >>>>
> > >>>
> https://confluence.atlassian.com/display/BITBUCKET/Linking+Bitbucket+and+GitHub+accounts+to+JIRA
> > >>>
> > >>>> ).
> > >>>>>>>>>>>
> > >>>>>>>>>>>> But I need both: Jira administration
rights to configure
> > >>>>>>>>>>>>
> > >>>>>>>>>>> the
> > >>>
> > >>>> plugin
> > >>>>>>>
> > >>>>>>>> and
> > >>>>>>>>>
> > >>>>>>>>>> GitHub password for "apache" user. Or I
missed something
> > >>>>>>>>>>>>
> > >>>>>>>>>>> and we
> > >>>>>
> > >>>>>> can't
> > >>>>>>>
> > >>>>>>>> use
> > >>>>>>>>>>
> > >>>>>>>>>>> this plugin at Apache infrastructure?
> > >>>>>>>>>>>> Maybe someone can suggest another
solution?
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Thanks,
> > >>>>>>>>>>>> Artem.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>
> > >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message