marklogic-community / grove-ml-gradle

Other
1 stars 2 forks source link

Feature/grove 424 #2

Open maeisabelle opened 5 years ago

maeisabelle commented 5 years ago

This is now ready for code review and merging:

Changes include following the suggested approach in https://github.com/derms/ml-gradle/tree/dev/examples/disconnected-project-using-plugins-and-gradlew with minor changes. I also updated the versions of some gradle dependencies to latest.

I also tested changing the new gradle properties I added for offline use (for choosing the gradle home, offline directory, generated zip directory and filename, etc). I also tested trying to deploy the zip from a different directory and deploying offline from the unzipped package.

grtjn commented 5 years ago

Great stuff so far!

Couple of small details:

maeisabelle commented 5 years ago

For the first bullet, I followed https://github.com/derms/ml-gradle/blob/dev/examples/disconnected-project-using-plugins-and-gradlew/gradle/wrapper/gradle-wrapper.properties. If not, it will try to download the gradle library if the gradle.user.home is set to relative path. Otherwise, there will be no problem.

Working on the 2nd bullets and 3rd bullet.

Great stuff so far!

Couple of small details:

  • I see you touched gradle-wrapper.. did you just pull in a newer version, or did you have to make changes to it yourself?
  • I'd like to get rid of -Pdisconnected=true, and use the official gradle arg --offline instead. I think I used it in my attempts, which I shared in comments. If not, ping me offline..
  • provide zip base name only, and append timestamp and/or version number (from package.json?), so you can use it to build a release package :)
grtjn commented 5 years ago

Ah, good to know. Perhaps good to dig up some docs on the wrapper to make sure we follow official guidelines..

And no rush!

grtjn commented 5 years ago

On second thought, can you support both --offline and -Pdisconnected=true. The benefit of the latter would be that you could use local props to pin down a certain copy to disconnected use permanently..

grtjn commented 5 years ago

FYI: I have run tests with this on a real offline project, and seemed pretty much spot on. I need to compare notes to make sure though (might take some time)..

grtjn commented 5 years ago

changed base to development..

grtjn commented 5 years ago

hoping to find time to review this PR next week..

grtjn commented 4 years ago

Gained a lot of experience from first hand with this in a project. I should really make time to take a close look at this, to polish a couple of things. Most just to make sure it works equally well for both online and offline cases.