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 <dma...@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.goncha...@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.goncha...@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 <c...@apache.org>: > >>>> > >>>> On Thu, Aug 13, 2015 at 05:54PM, Dmitriy Setrakyan wrote: > >>>>> > >>>>>> On Thu, Aug 13, 2015 at 5:51 PM, Konstantin Boudnik <c...@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 <ashu...@gridgain.com>: > >>>>>>>> > >>>>>>>> Inline. > >>>>>>>>> > >>>>>>>>> On Wed, Aug 12, 2015 at 10:19 AM, Dmitriy Setrakyan < > >>>>>>>>> > >>>>>>>> dsetrak...@apache.org > >>>>>>> > >>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> On Tue, Aug 11, 2015 at 6:28 AM, Artem Shutak < > >>>>>>>>>> > >>>>>>>>> ashu...@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 < > >>>>>>>>>>> > >>>>>>>>>> ashu...@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. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>> > >
