[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Code Committ Process for Fineract Committers


Hi all -

I am working this morning to get all of the integration tests back up and
running, which will facilitate merging of open PRs. I will let you know
when I have that done - hopefully today.

Thanks,
Steve


On Mon, Aug 20, 2018 at 9:12 AM Mexina Daniel <mexina@singo.africa> wrote:

> Hello Myrle
>
> Thank you for the more clarifications you have provided.
>
> For now, is it okay to merge the PRs i have tested and find they work just
> fine in the application and there is no need to run integration tests?
>
> Because there are un-merged PRs that are needed in the community.
>
> Regards
>
> ----- Original Message -----
> From: "Myrle Krantz" <myrle@xxxxxxxxxx>
> To: "dev" <dev@xxxxxxxxxxxxxxxxxxx>
> Sent: Monday, August 20, 2018 3:36:03 PM
> Subject: Re: Code Committ Process for Fineract Committers
>
> Hi Mexina,
>
> Coming back to your questions
>
> 1.) Theoretically we should be making sure all the integration tests
> work before we merge a PR.  But we currently have no continuous
> integration (CI) processes in place to build or run those integration
> tests which means that it's up to individual developers to do it
> themselves.  Hence it is possible for a committer to merge something
> which breaks an integration test.  For this reason I suggest running
> the integration tests on the changeset you want to merge your changes
> with to get a baseline.  This is an area we need to do better in, and
> I welcome anyone who wants to put in the time.
>
> By the way, the attachment you mentioned didn't reach the mailing
> list.  A lot of people use pastebin and post links, but I'm not a fan
> of that approach since that stuff doesn't get archived.  For pure text
> files, you may be able to copy/paste out the interesting parts into
> your email.  Whatever approach you chose, if you want to be sure that
> what you sent also reached the mailing list, you can check the
> archives here: https://lists.apache.org/list.html?dev@xxxxxxxxxxxxxxxxxxx
>
> 2.) You've already found the answer to your question, but I wanted to
> make sure it was clear.  The command you were looking for in the case
> of pull request 444 was:
> git pull https://github.com/wkk91193/incubator-fineract
> extend-mifos-data-import-tool-branch-all-modules
>
> Basically you're telling git where to get the code from.  In this case
> it's in the github account of the user who created the pull request.
> This is not a standard case either since the its in a repo in that
> account which has a slightly different name than the repo the pull
> request was made to.  The second parameter to git pull is the name of
> the branch within that repository being pulled from.
>
> I hope that helps.  I'm going to try to find a way to make this less
> of a stumbling block.
>
> I'll be more responsive for the coming week.  If you want to ask me
> questions on the list, now is a good time.
>
> Best Regards,
> Myrle
>
> On Mon, Aug 6, 2018 at 2:35 PM Mexina Daniel <mexina@singo.africa> wrote:
> >
> > Hello Steve
> >
> > See the attachment, is the result i get when i run integrationTest
> >
> > What is a work around of this?
> >
> > Regards
> >
> > ----- Original Message -----
> > From: "Mexina Daniel" <mexina@singo.africa>
> > To: "sconrad1" <sconrad1@xxxxxxxxx>
> > Cc: "Myrle Krantz" <myrle@xxxxxxxxxx>, "Avik Ganguly" <
> avikganguly010@xxxxxxxxx>, "edcable" <edcable@xxxxxxxxx>, "Isaac Kamga" <
> isaac.kamga@xxxxxxxxx>, "santosh" <santosh@xxxxxxxxxxxxxxxxxxxxxxx>,
> "Nazeer Hussain Shaik" <nazeerhussain.shaik@xxxxxxxxx>, "nayan ambali" <
> nayan.ambali@xxxxxxxxx>, "shruthi" <shruthi@xxxxxxxxxxxxxxxxxxxxxxx>,
> "dev" <dev@xxxxxxxxxxxxxxxxxxx>
> > Sent: Thursday, August 2, 2018 10:16:39 AM
> > Subject: Re: Code Committ Process for Fineract Committers
> >
> > Hello Steve
> >
> > This was really helpfully, thank you
> >
> > The next question will be, how can i work around it so that i can merge
> this PR that seems to fix some of the issues and other PRs that are really
> needed to be merged.
> >
> > Best Regards
> >
> >
> > ----- Original Message -----
> > From: "Steve Conrad" <sconrad1@xxxxxxxxx>
> > To: "dev" <dev@xxxxxxxxxxxxxxxxxxx>
> > Cc: "Myrle Krantz" <myrle@xxxxxxxxxx>, "Avik Ganguly" <
> avikganguly010@xxxxxxxxx>, "edcable" <edcable@xxxxxxxxx>, "Isaac Kamga" <
> isaac.kamga@xxxxxxxxx>, "santosh" <santosh@xxxxxxxxxxxxxxxxxxxxxxx>,
> "Nazeer Hussain Shaik" <nazeerhussain.shaik@xxxxxxxxx>, "nayan ambali" <
> nayan.ambali@xxxxxxxxx>, "shruthi" <shruthi@xxxxxxxxxxxxxxxxxxxxxxx>
> > Sent: Wednesday, August 1, 2018 6:02:26 PM
> > Subject: Re: Code Committ Process for Fineract Committers
> >
> > Hi Mexina -
> >
> > I did run into some issues with Integration Tests as well when I was
> > merging - it seems that some of the setup data files are configured for
> > Windows paths. I was able to work around that, but we should have someone
> > look into cleaning up the integration tests so that they run more easily.
> >
> > As far as your specific question about the merge process, it can be a bit
> > confusing. As the instructions indicate, you will create a new new branch
> > in your cloned repo In this case, the PR doesn't have a ticket name. @Ed
> > Cable <edcable@xxxxxxxxx> should we require any PRs to be attached to a
> > ticket? Otherwise, the name of the branch would be something like
> > wkk91193-bulk-impoort-fix-importhandler.
> >
> > It is important that whenever someone wants to submit a PR, they create a
> > branch that contains only the changes that are to be merged. In this
> case,
> > the forked repo name is  wkk91193 and the remote branch name is
> > extend-mifos-data-import-tool-branch-all-modules That is how you get only
> > the changes that are specific to that PR.
> >
> > Hope that helps clarify. Please let me know if you have other questions.
> > Thanks,
> > Steve
> >
> > On Wed, Aug 1, 2018 at 9:18 AM Mexina Daniel <mexina@singo.africa>
> wrote:
> >
> > > Hello Myrle
> > >
> > > Thank you for the clarifications you have provided,
> > >
> > > I also was experiencing some challenges during merging of PRs,
> > >
> > > 1. Don't all integration tests has to be successfully for a committer
> to
> > > go on with merging?, i see Steve was able to merge but when i run
> > > integrationTests in my end there are some which still fails.
> > >
> > > 2. When i was trying to merge the PRs which fix the integratioTest
> > > https://github.com/apache/fineract/pull/444 which failed by
> instructions
> > > from
> > >
> https://cwiki.apache.org/confluence/display/FINERACT/Merge+a+pull+request+to+a+mirrored+Apache+repo
> > > .... On step 3 of merging pull request, i don't understand the command
> > >
> > > " > git pull https://github.com/<forked-reponame>/fineract.git
> > > <remote-branch-name>"
> > >
> > > How does it exactly pull from specific PR, unless am misunderstanding
> the
> > > variable "forked-reponame" and "remote-branch-name"
> > >
> > > Can you help?
> > >
> > > Best Regards
> > >
> > > ----- Original Message -----
> > > From: "Myrle Krantz" <myrle@xxxxxxxxxx>
> > > To: "dev" <dev@xxxxxxxxxxxxxxxxxxx>
> > > Cc: "Avik Ganguly" <avikganguly010@xxxxxxxxx>, "edcable" <
> > > edcable@xxxxxxxxx>, "Isaac Kamga" <isaac.kamga@xxxxxxxxx>, "santosh" <
> > > santosh@xxxxxxxxxxxxxxxxxxxxxxx>, "Nazeer Hussain Shaik" <
> > > nazeerhussain.shaik@xxxxxxxxx>, "nayan ambali" <nayan.ambali@xxxxxxxxx
> >,
> > > shruthi@xxxxxxxxxxxxxxxxxxxxxxx
> > > Sent: Saturday, July 28, 2018 5:57:31 PM
> > > Subject: Re: Code Committ Process for Fineract Committers
> > >
> > > Hey Ed,
> > >
> > > All Fineract committers have access rights to change all Fineract
> > > repositories. They can merge their own or others PRs. Nazeer, Nayan,
> Steve,
> > > Santosh, Ed, Avik are all committers. There are a total of 20+
> committers.
> > >
> > > The process for committing changes is different depending on the
> > > repository. We have two kinds of repositories. Mirrored repositories
> and
> > > gitbox repositories.
> > >
> > > Mirrored repositories are:
> > > ————
> > > fineract-site
> > > fineract
> > >
> > > As Steve pointed out, mirrored repositories are changed via the process
> > > described here:
> > >
> > >
> > >
> https://cwiki.apache.org/confluence/display/FINERACT/Merge+a+pull+request+to+a+mirrored+Apache+repo
> > >
> > > I personally find that process painful. It is possible that someone
> tried
> > > that, failed at it, and came to the mistaken conclusion that he didn’t
> have
> > > access rights.
> > >
> > >
> > > Gitbox repositories are:
> > > —————-
> > > All the Fineract CN repositories
> > >
> > > (We are in the process of changing fineract-site to a gitbox
> repository.)
> > >
> > > In gitbox repositories, PRs can be merged via the Github UI most of us
> are
> > > familiar with. In November of last year, I explained what you need to
> do to
> > > enable this here on the mailing list:
> > >
> > >
> > >
> https://lists.apache.org/thread.html/dec82c43eddd37b2e90020120e285d5cffe04f1fe448e1dfa1b9c49b@%3Cdev.fineract.apache.org%3E
> > >
> > > If you are a committer and have tried to merge a change-set and failed,
> > > don’t suffer in silence. You are allowed to make changes. Tell us
> you’re
> > > struggling so we can help you. I promise to never say RTFM to a
> committer.
> > > And if someone else does it, I’ll call them on it.
> > >
> > > Best Regards,
> > > Myrle
> > >
> > >
> > >
> > >
> > > On Fri 27. Jul 2018 at 19:08 Steve Conrad <sconrad1@xxxxxxxxx> wrote:
> > >
> > > > Hi all,
> > > >
> > > > I was able to merge PRs using the process described in this document:
> > > >
> > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/FINERACT/Merge+a+pull+request+to+a+mirrored+Apache+repo
> > > >
> > > > Myrle created this doc and I made a few updates to it.
> > > >
> > > > However, it may be that we need to use this GitBox process moving
> > > forward.
> > > > I did just go through the process that Ed describes above and it's
> quite
> > > > easy. I had to go to id.apache.org and set my GitHub user id and
> then
> > > > accept the invitation that came from ASF. You can check your GitHub
> > > > organizations in Settings->Organizations. You also need to have 2FA
> > > enabled
> > > > on your GitHub account.
> > > >
> > > > Thanks,
> > > > Steve
> > > >
> > > > On Fri, Jul 27, 2018 at 10:54 AM Ed Cable <edcable@xxxxxxxxx> wrote:
> > > >
> > > > > Hello all,
> > > > >
> > > > > @Shruthi M R <shruthi@xxxxxxxxxxxxxxxxxxxxxxx>  was working with
> some
> > > of
> > > > > the committers to get the recent self-service APIs she created
> merged
> > > > into
> > > > > the code base. Both @Nazeer Hussain Shaik <
> > > nazeerhussain.shaik@xxxxxxxxx
> > > > >
> > > > >   and @Nayan Ambali <nayan.ambali@xxxxxxxxx>  didn't think they
> had
> > > > > permission to merge the code so I think there's confusion
> regarding the
> > > > > code commit process for committers that I wanted to share on list.
> > > > >
> > > > > Pull requests aren't actually merged through Github but rather
> through
> > > > > Gitbox as described at https://gitbox.apache.org/setup/
> > > > >
> > > > > Necessary prerequisites for committers to have the above setup
> work is
> > > > > being added to the Fineract committer roster, having your GitHub ID
> > > > > associated with ASF ID, and 2FA enabled.
> > > > >
> > > > > @Steve Conrad <sconrad1@xxxxxxxxx> , @Avik Ganguly
> > > > > <avikganguly010@xxxxxxxxx>  could you give feedback to others as I
> > > know
> > > > > you recently went through this process.
> > > > >
> > > > > @Isaac Kamga <isaac.kamga@xxxxxxxxx> , @Santosh Math
> > > > > <santosh@xxxxxxxxxxxxxxxxxxxxxxx>  - could you work on improving
> > > > > documentation on Fineract to reflect this process for committers?
> > > > >
> > > > > Would you also be able to try and merge these incoming pull
> requests?
> > > See
> > > > > below:
> > > > >
> > > > > https://github.com/apache/fineract/pull/461 ------ Fineract -611
> > > unable
> > > > > to create share product
> > > > >
> > > > > https://github.com/apache/fineract/pull/463  ------  Fineract- 628
> > > > > Savings account Api for Self Service App
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Ed
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > *Ed Cable*
> > > > > President/CEO, Mifos Initiative
> > > > > edcable@xxxxxxxxx | Skype: edcable | Mobile: +1.484.477.8649
> > > > >
> > > > > *Collectively Creating a World of 3 Billion Maries | *
> http://mifos.org
> > > > > <http://facebook.com/mifos>  <http://www.twitter.com/mifos>
> > > > >
> > > > >
> > > >
> > > --
> > > Mexina M Daniel
> > > Lead Software Developer
> > > Research & Development
> > >
> > > Office: +255 22 261 8511 | Mobile: +255 712 110 791
> > >
> > > Singo Africa Limited
> > > Block G, Mbezi Beach B | 7Nakawale Road | P.O BOX 78908 | 14121 Dar es
> > > salaam
> > >
> > > singo.africa | amala.co.tz
> > >
> > > Let's grow together
> > >
> > --
> > Mexina M Daniel
> > Lead Software Developer
> > Research & Development
> >
> > Office: +255 22 261 8511 | Mobile: +255 712 110 791
> >
> > Singo Africa Limited
> > Block G, Mbezi Beach B | 7Nakawale Road | P.O BOX 78908 | 14121 Dar es
> > salaam
> >
> > singo.africa | amala.co.tz
> >
> > Let's grow together
> > --
> > Mexina M Daniel
> > Lead Software Developer
> > Research & Development
> >
> > Office: +255 22 261 8511 | Mobile: +255 712 110 791
> >
> > Singo Africa Limited
> > Block G, Mbezi Beach B | 7Nakawale Road | P.O BOX 78908 | 14121 Dar es
> > salaam
> >
> > singo.africa | amala.co.tz
> >
> > Let's grow together
> --
> Mexina M Daniel
> Lead Software Developer
> Research & Development
>
> Office: +255 22 261 8511 | Mobile: +255 712 110 791
>
> Singo Africa Limited
> Block G, Mbezi Beach B | 7Nakawale Road | P.O BOX 78908 | 14121 Dar es
> salaam
>
> singo.africa | amala.co.tz
>
> Let's grow together
>