Adds Support For Generating Maven Package For Android Library #58

thorbenprimke · 2018-02-28T16:12:45Z

Summary:
This adds support for generating the maven package similar to how
React Native core does it.
This allows a brownfield Android project reference this maven package
instead of the project in the node_modules.

This also adds a few new fields to package.json. Do y'all think it's okay to have placeholder values or should I add the ability to provide them to the cli?

thorbenprimke · 2018-03-03T17:29:38Z

I have updated this PR with config options for the author's github, name and email.
The README is updated as well.

Testing:

Verified that placeholders are used with: ./react-native-create-library/cli.js AweDsdTest1
Verified that actual values are used with: ./react-native-create-library/cli.js AweDsdTest1 --author-github=thorbenprimke --author-name='Thorben Primke' --author-email='first@lastname.com'

frostney

Great pull request 👍
Looking at this, it would be great to have an interactive mode. Obviously totally out of the scope of this pull request, but do what you think about this idea in general?

frostney · 2018-03-12T22:07:47Z

templates/general.js

    return `
 {
  "name": "${moduleName}",
+  "title": "${moduleName.split('-').map(word => word[0].toUpperCase() + word.substr(1)).join(' ')}",


I'm wondering if we need this property here or if we could prepare this in the templates/android.js file instead?

Actually, I placed it into package.json on purpose. It may only be used by the Android maven config at this time, but for the sake of consistency I think it's good to have it here so that all config values that feed into the maven config can be changed in one place.

frostney · 2018-03-12T22:08:00Z

templates/general.js

+    "name": "${authorName}",
+    "email": "${authorEmail}"
+  },
+  "license": "Apache-2.0",


Nitpick: Do we need to hardcode the license? Could we get that as a command-line parameter or default value as well?

Sure. Fair point I'll make it configurble as well. 💯

frostney · 2018-03-12T22:10:01Z

templates/android.js

 }

 repositories {
+    maven {


This changeset is also in #51, right? 🙂

Yes, it should be gone after #51 or #39 is merged and I rebase. It's necessary in order for the project to build.

frostney · 2018-03-12T22:12:22Z

lib.js

 const DEFAULT_PLATFORMS = ['android', 'ios', 'windows'];
 const DEFAULT_OVERRIDE_PREFIX = false;
+const DEFAULT_AUTHOR_GITHUB = 'github_account_name'
+const DEFAULT_AUTHOR_NAME = 'Your Name'


This might be out of the scope of this pull request, but do you think it would be possible to get the default values from the global Git config?

Yea, probably out of scope for this PR but I don't see a reason why that is not possible. Basically we could first default to the git config --global user.name (as well as email) and if those are not set, then only use these secondary default values.

I agree we should file an issue for it and follow up. Would be a good starter task too imho.

thorbenprimke

Thanks for the suggestions @frostney - I'll get this PR updated.

thorbenprimke · 2018-03-12T23:00:53Z

lib.js

 const DEFAULT_PLATFORMS = ['android', 'ios', 'windows'];
 const DEFAULT_OVERRIDE_PREFIX = false;
+const DEFAULT_AUTHOR_GITHUB = 'github_account_name'
+const DEFAULT_AUTHOR_NAME = 'Your Name'


Yea, probably out of scope for this PR but I don't see a reason why that is not possible. Basically we could first default to the git config --global user.name (as well as email) and if those are not set, then only use these secondary default values.

thorbenprimke · 2018-03-12T23:02:32Z

templates/android.js

 }

 repositories {
+    maven {


Yes, it should be gone after #51 or #39 is merged and I rebase. It's necessary in order for the project to build.

thorbenprimke · 2018-03-12T23:05:03Z

templates/general.js

    return `
 {
  "name": "${moduleName}",
+  "title": "${moduleName.split('-').map(word => word[0].toUpperCase() + word.substr(1)).join(' ')}",


Actually, I placed it into package.json on purpose. It may only be used by the Android maven config at this time, but for the sake of consistency I think it's good to have it here so that all config values that feed into the maven config can be changed in one place.

thorbenprimke · 2018-03-12T23:05:22Z

templates/general.js

+    "name": "${authorName}",
+    "email": "${authorEmail}"
+  },
+  "license": "Apache-2.0",


Sure. Fair point I'll make it configurble as well. 💯

Summary: This adds support for generating the maven package similar to how React Native core does it. This allows a brownfield Android project reference this maven package instead of the project in the node_modules.

Updates README

thorbenprimke · 2018-03-14T04:27:25Z

@maicki - mind giving this PR a look?

maicki

LGTM

maicki · 2018-03-14T16:35:50Z

lib.js

 const DEFAULT_PLATFORMS = ['android', 'ios', 'windows'];
 const DEFAULT_OVERRIDE_PREFIX = false;
+const DEFAULT_AUTHOR_GITHUB = 'github_account_name'
+const DEFAULT_AUTHOR_NAME = 'Your Name'


I agree we should file an issue for it and follow up. Would be a good starter task too imho.

maicki · 2018-03-14T16:41:59Z

templates/general.js

  },
+  "repository": {
+    "type": "git",
+    "url": "git+https://github.com/${authorGithub}/${moduleName}.git",


What if the url for the repository is at a different place as at the authors github username. Thinking off such libs that are under a organization.

Yes, I considered that. Perhaps the naming should be changed to githubAccount or hostingGithubAccount.

I guess that brings up the next thing which is what if you use bitbucket or another service. I think this template / generator will serve most needs out of the box but in some cases a little modification may be necessary as it otherwise would make the options too complex.

thorbenprimke force-pushed the android_maven branch from 796d565 to e2d7a55 Compare March 2, 2018 19:15

frostney reviewed Mar 12, 2018

View reviewed changes

thorbenprimke commented Mar 12, 2018

View reviewed changes

thorbenprimke force-pushed the android_maven branch from 6b5c0f3 to 6846f0f Compare March 13, 2018 08:07

thorbenprimke added 3 commits March 13, 2018 21:10

Adds Support For Generating Maven Package For Android Library

65d8585

Summary: This adds support for generating the maven package similar to how React Native core does it. This allows a brownfield Android project reference this maven package instead of the project in the node_modules.

Adds config options for author's github, name and email.

162f308

Updates README

added default values to readme

00174da

thorbenprimke force-pushed the android_maven branch from 6846f0f to 00174da Compare March 14, 2018 04:10

thorbenprimke added 2 commits March 13, 2018 21:12

fixed merge conflict

d75a9bc

Makes the library license type command line configurable

59f398c

maicki approved these changes Mar 14, 2018

View reviewed changes

thorbenprimke mentioned this pull request Mar 14, 2018

Full Name And Email From Git Config #60

Open

Changes authorGithub to githubAccount

3932726

thorbenprimke mentioned this pull request Mar 17, 2018

Add Interactive Config Mode #61

Open

thorbenprimke merged commit db4549a into frostney:master Mar 17, 2018

thorbenprimke deleted the android_maven branch March 17, 2018 05:09

Adds Support For Generating Maven Package For Android Library #58

Adds Support For Generating Maven Package For Android Library #58

Uh oh!

Conversation

thorbenprimke commented Feb 28, 2018

Uh oh!

thorbenprimke commented Mar 3, 2018

Uh oh!

frostney left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorbenprimke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorbenprimke commented Mar 14, 2018

Uh oh!

maicki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants