Upgrade Gradle version used in Fineract

[Michael Vorburger <mi...@vorburger.ch>]

Hello,

I would like to propose https://github.com/apache/fineract/pull/525 to
upgrade the Gradle version used in Fineract.

It would be good to get some reviews and perhaps manual testing from some
people. I does of course build again (see the "green" PR, now that we have
Travis CI integration), but I have not done any testing of it... would
someone like pull it, build it, fire it up and give it a try to make sure I
didn't break anything?

Please see https://issues.apache.org/jira/browse/FINERACT-700 for the full
details of the work that was required and done.

I'll self merge this in 2 weeks if I don't hear any objections from anyone.

Regards,
Michael

PS: I'm not actually reading this list (and won't, just due to general
load), but am replying to this via web interface; please Reply-To-All if
you want me to see replies.
_______________________
Michael Vorburger
http://www.vorburger.ch

Re: Upgrade Gradle version used in Fineract

[Vishwas Babu <vi...@confluxtechnologies.com>]

On Fri, Feb 22, 2019 at 12:01 AM Juhan Aasaru <aa...@gmail.com> wrote:

> Hi!
>
> Vishwas and Myrle, thanks for explanations regarding Gradle version and
> wrapper download instructions.
> I first didn't notice that Michael had changed Gradle version inside
> gradle-wrapper.properties
> Instructions regarding downloading wrapper could include a line:
> "You only need to download wrapper if
> file fineract-provider/gradle/wrapper/gradle-wrapper.jar doesn't already
> exist"
>

+1 . This change has now been made with the commit
https://github.com/apache/fineract/commit/8b14bb88a7528e634ee1c33f69ccb2a9e3e3da71


>
> I had problems with Gradle when I first imported the project.
> I think some of the confusion was caused by the fact that root directory
> contains gradlew and gradlew.bat but the wrapper is
> inside fineract-provider/gradle/wrapper
>
> When I imported the project into IntelliJ IDEA it auto-generated following
> files:
> <root>/gradle/wrapper/gradle-wrapper.jar
> <root>/gradle/wrapper/gradle-wrapper.properties
> <root>/fineract-provider/gradlew
> <root>/fineract-provider/gradlew.bat
>
> This is caused by the fact that the actual root directory of Gradle project
> is fineract-provider
> My proposition would be to include separate gradlew and gradlew.bat in
> fineract-provider directory
>

+1 . It looks like gradlew and gradlew.bat were deliberately moved out from
under /fineract-provider to the project root as a prerequisite for the
migration to Apache. Another option might be to check-in relative symlinks
for gradlew and gradlew.bat (pretty sure git handles it fine, not too sure
about IDE's though)?

(to avoid them being auto generated or considered missing). Or completely
> move them from root to fineract-provider.


> Kind regards
> Juhan
>
>
> Kontakt Myrle Krantz (<my...@apache.org>) kirjutas kuupäeval E, 18.
> veebruar 2019 kell 22:59:
>
> > On Mon, Feb 18, 2019 at 8:39 PM Vishwas Babu <
> > vishwas@confluxtechnologies.com> wrote:
> >
> > > @Juhan : While I am not sure why the readme has instructions for
> > > downloading gradle wrapper separately (given that it is a part of the
> > > project repo and committed into the code),
> > >
> >
> > ASF policy is that releases should not include binaries.  That includes
> the
> > gradle.jar.  So before a release is cut the gradle jar is deleted.  Many
> > developers work from forks.  We include the gradlel jar for them.  But we
> > don't want two versions of the readme.  That makes it necessary to
> download
> > the gradle wrapper or install gradle locally, and necessary to include
> that
> > in the instructions.
> >
> > Best Regards,
> > Myrle
> >
>

Re: Upgrade Gradle version used in Fineract

[Juhan Aasaru <aa...@gmail.com>]

Hi!

Vishwas and Myrle, thanks for explanations regarding Gradle version and
wrapper download instructions.
I first didn't notice that Michael had changed Gradle version inside
gradle-wrapper.properties
Instructions regarding downloading wrapper could include a line:
"You only need to download wrapper if
file fineract-provider/gradle/wrapper/gradle-wrapper.jar doesn't already
exist"

I had problems with Gradle when I first imported the project.
I think some of the confusion was caused by the fact that root directory
contains gradlew and gradlew.bat but the wrapper is
inside fineract-provider/gradle/wrapper

When I imported the project into IntelliJ IDEA it auto-generated following
files:
<root>/gradle/wrapper/gradle-wrapper.jar
<root>/gradle/wrapper/gradle-wrapper.properties
<root>/fineract-provider/gradlew
<root>/fineract-provider/gradlew.bat

This is caused by the fact that the actual root directory of Gradle project
is fineract-provider
My proposition would be to include separate gradlew and gradlew.bat in
fineract-provider directory
(to avoid them being auto generated or considered missing). Or completely
move them from root to fineract-provider.

Kind regards
Juhan


Kontakt Myrle Krantz (<my...@apache.org>) kirjutas kuupäeval E, 18.
veebruar 2019 kell 22:59:

> On Mon, Feb 18, 2019 at 8:39 PM Vishwas Babu <
> vishwas@confluxtechnologies.com> wrote:
>
> > @Juhan : While I am not sure why the readme has instructions for
> > downloading gradle wrapper separately (given that it is a part of the
> > project repo and committed into the code),
> >
>
> ASF policy is that releases should not include binaries.  That includes the
> gradle.jar.  So before a release is cut the gradle jar is deleted.  Many
> developers work from forks.  We include the gradlel jar for them.  But we
> don't want two versions of the readme.  That makes it necessary to download
> the gradle wrapper or install gradle locally, and necessary to include that
> in the instructions.
>
> Best Regards,
> Myrle
>

Re: Upgrade Gradle version used in Fineract

[Myrle Krantz <my...@apache.org>]

On Mon, Feb 18, 2019 at 8:39 PM Vishwas Babu <
vishwas@confluxtechnologies.com> wrote:

> @Juhan : While I am not sure why the readme has instructions for
> downloading gradle wrapper separately (given that it is a part of the
> project repo and committed into the code),
>

ASF policy is that releases should not include binaries.  That includes the
gradle.jar.  So before a release is cut the gradle jar is deleted.  Many
developers work from forks.  We include the gradlel jar for them.  But we
don't want two versions of the readme.  That makes it necessary to download
the gradle wrapper or install gradle locally, and necessary to include that
in the instructions.

Best Regards,
Myrle

Re: Upgrade Gradle version used in Fineract

[Vishwas Babu <vi...@confluxtechnologies.com>]

Hi Mike,

One issue would be the removal of TaskInternal.execute() method in Gradle
5.x which is used in all DB related tasks (explicit call to
flywayMigrate.execute() etc).

@Juhan : While I am not sure why the readme has instructions for
downloading gradle wrapper separately (given that it is a part of the
project repo and committed into the code), Mike's pr does update the gradle
version in the checked in wrapper and should be sufficient to ensure that
all developers are using the right gradle version. What you are suggesting
is upgrading the wrapper itself, which while undoubtedly would be good to
have, probably isn't a must do for this pr given that there do not seem to
be any major changes to the features of the wrapper itself.

Details of a quick fix I attempted to work around the removed
TaskInternal.execute() method follow.

Using finalizedBy instead of execute ( i.e migrateTenantListDB.finalizedBy
flywayMigrate) led to the error "Could not get unknown property
'classesDir' for integration test classes of type
org.gradle.api.internal.tasks.DefaultSourceSetOutput". This was fixed by
upgrading flyway plugin to.

apply plugin: 'org.flywaydb.flyway'
    dependencies {
        classpath
"gradle.plugin.com.boxfuse.client:gradle-plugin-publishing:5.2.4"
    }
}

However, this does not seem to work with the current drizzle driver on the
classpath (results in the error "MySQL upgrade required: MySQL 0.1 is
outdated and no longer supported by Flyway. Flyway currently supports MySQL
5.1 and newer"). It runs without issues if I swap in a MySQL driver though.

Regards,
Vishwas


On Mon, Feb 18, 2019 at 1:41 AM Juhan Aasaru <aa...@gmail.com> wrote:

> Hello Mike!
>
> That is a great initiative. I checked the pull request and I noticed
> something that you might want to take into account.
>
> Currently, Readme.md has instructions on how to download Gradle Wrapper.
> In my understanding, the correct Gradle Wrapper should be already committed
> to the code and the
> developers shouldn't need to download the Wrapper. This is the whole idea
> behind using Gradle Wrapper
> so that everybody uses the same Gradle version.
>
> I see that the old wrapper is still there under
> /fineract-provider/gradle/wrapper
> and your pull request doesn't touch that.
>
> I think the issue could be solved by:
> 1. run gradle wrapper
> 2. include the changed files (/fineract-provider/gradle/wrapper) in your
> pull request
> 3. update Readme.md and remove the instructions about needing to download
> Gradle Wrapper.
>
> I didn't try it out myself so it might need some changes but I hope you get
> the idea.
>
> Kind regards,
> Juhan
>
>
>
>
> Kontakt Michael Vorburger (<mi...@vorburger.ch>) kirjutas kuupäeval E, 18.
> veebruar 2019 kell 01:51:
>
> > Hello,
> >
> > I would like to propose https://github.com/apache/fineract/pull/525 to
> > upgrade the Gradle version used in Fineract.
> >
> > It would be good to get some reviews and perhaps manual testing from some
> > people. I does of course build again (see the "green" PR, now that we
> have
> > Travis CI integration), but I have not done any testing of it... would
> > someone like pull it, build it, fire it up and give it a try to make
> sure I
> > didn't break anything?
> >
> > Please see https://issues.apache.org/jira/browse/FINERACT-700 for the
> full
> > details of the work that was required and done.
> >
> > I'll self merge this in 2 weeks if I don't hear any objections from
> anyone.
> >
> > Regards,
> > Michael
> >
> > PS: I'm not actually reading this list (and won't, just due to general
> > load), but am replying to this via web interface; please Reply-To-All if
> > you want me to see replies.
> > _______________________
> > Michael Vorburger
> > http://www.vorburger.ch
> >
>

Re: Upgrade Gradle version used in Fineract

[Juhan Aasaru <aa...@gmail.com>]

Hello Mike!

That is a great initiative. I checked the pull request and I noticed
something that you might want to take into account.

Currently, Readme.md has instructions on how to download Gradle Wrapper.
In my understanding, the correct Gradle Wrapper should be already committed
to the code and the
developers shouldn't need to download the Wrapper. This is the whole idea
behind using Gradle Wrapper
so that everybody uses the same Gradle version.

I see that the old wrapper is still there under
/fineract-provider/gradle/wrapper
and your pull request doesn't touch that.

I think the issue could be solved by:
1. run gradle wrapper
2. include the changed files (/fineract-provider/gradle/wrapper) in your
pull request
3. update Readme.md and remove the instructions about needing to download
Gradle Wrapper.

I didn't try it out myself so it might need some changes but I hope you get
the idea.

Kind regards,
Juhan




Kontakt Michael Vorburger (<mi...@vorburger.ch>) kirjutas kuupäeval E, 18.
veebruar 2019 kell 01:51:

> Hello,
>
> I would like to propose https://github.com/apache/fineract/pull/525 to
> upgrade the Gradle version used in Fineract.
>
> It would be good to get some reviews and perhaps manual testing from some
> people. I does of course build again (see the "green" PR, now that we have
> Travis CI integration), but I have not done any testing of it... would
> someone like pull it, build it, fire it up and give it a try to make sure I
> didn't break anything?
>
> Please see https://issues.apache.org/jira/browse/FINERACT-700 for the full
> details of the work that was required and done.
>
> I'll self merge this in 2 weeks if I don't hear any objections from anyone.
>
> Regards,
> Michael
>
> PS: I'm not actually reading this list (and won't, just due to general
> load), but am replying to this via web interface; please Reply-To-All if
> you want me to see replies.
> _______________________
> Michael Vorburger
> http://www.vorburger.ch
>