[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[
https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16671107#comment-16671107
]
ASF GitHub Bot commented on CB-14201:
-
dpogue closed pull request #465: CB-14201: (android) Removes Gradle property
in-line command arguments…
URL: https://github.com/apache/cordova-android/pull/465
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/bin/templates/cordova/lib/builders/ProjectBuilder.js
b/bin/templates/cordova/lib/builders/ProjectBuilder.js
index b000de3e0..24aef07cd 100644
--- a/bin/templates/cordova/lib/builders/ProjectBuilder.js
+++ b/bin/templates/cordova/lib/builders/ProjectBuilder.js
@@ -46,20 +46,15 @@ class ProjectBuilder {
} else if (cmd === 'debug') {
cmd = 'cdvBuildDebug';
}
-var args = [cmd, '-b', path.join(this.root, 'build.gradle')];
+
+let args = [cmd, '-b', path.join(this.root, 'build.gradle')];
+
if (opts.arch) {
args.push('-PcdvBuildArch=' + opts.arch);
}
-// 10 seconds -> 6 seconds
-args.push('-Dorg.gradle.daemon=true');
-// to allow dex in process
-args.push('-Dorg.gradle.jvmargs=-Xmx2048m');
-// allow NDK to be used - required by Gradle 1.5 plugin
-// args.push('-Pandroid.useDeprecatedNdk=true');
args.push.apply(args, opts.extraArgs);
-// Shaves another 100ms, but produces a "try at own risk" warning. Not
worth it (yet):
-// args.push('-Dorg.gradle.parallel=true');
+
return args;
}
diff --git a/bin/templates/cordova/lib/config/GradlePropertiesParser.js
b/bin/templates/cordova/lib/config/GradlePropertiesParser.js
new file mode 100644
index 0..ac7f38634
--- /dev/null
+++ b/bin/templates/cordova/lib/config/GradlePropertiesParser.js
@@ -0,0 +1,97 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+let fs = require('fs');
+let path = require('path');
+let propertiesParser = require('properties-parser');
+let events = require('cordova-common').events;
+
+class GradlePropertiesParser {
+/**
+* Loads and Edits Gradle Properties File.
+*
+* @param {String} platformDir is the path of the Android platform directory
+*/
+constructor (platformDir) {
+this._defaults = {
+// 10 seconds -> 6 seconds
+'org.gradle.daemon': 'true',
+
+// to allow dex in process
+'org.gradle.jvmargs': '-Xmx2048m',
+
+// allow NDK to be used - required by Gradle 1.5 plugin
+'android.useDeprecatedNdk': 'true'
+
+// Shaves another 100ms, but produces a "try at own risk" warning.
Not worth it (yet):
+// 'org.gradle.parallel': 'true'
+};
+
+this.gradleFilePath = path.join(platformDir, 'gradle.properties');
+}
+
+configure () {
+events.emit('verbose', '[Gradle Properties] Preparing Configuration');
+
+this._initializeEditor();
+this._configureDefaults();
+this._save();
+}
+
+/**
+ * Initialize the properties editor for parsing, setting, etc.
+ */
+_initializeEditor () {
+// Touch empty gradle.properties file if missing.
+if (!fs.existsSync(this.gradleFilePath)) {
+events.emit('verbose', '[Gradle Properties] File missing, creating
file with Cordova defaults.');
+fs.writeFileSync(this.gradleFilePath, '', 'utf-8');
+}
+
+// Create an editor for parsing, getting, and setting configurations.
+this.gradleFile = propertiesParser.createEditor(this.gradleFilePath);
+}
+
+/**
+ * Validate that defaults are set and set the missing defaults.
+ */
+_configureDefaults () {
+// Loop though Cordova default properties and set only if missing.
+Object.keys(this._defaults).forEach(key => {
+let value = this.gradleFile.get(key);
+
+
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16671108#comment-16671108 ] ASF subversion and git services commented on CB-14201: -- Commit dacb0e5c721a7476b767261433fb96e7a7b6f088 in cordova-android's branch refs/heads/master from エリス [ https://gitbox.apache.org/repos/asf?p=cordova-android.git;h=dacb0e5 ] CB-14201: (android) Removes Gradle property in-line command arguments for gradle.properties > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16671110#comment-16671110 ] ASF subversion and git services commented on CB-14201: -- Commit e58453d3e6f16e45fb9ac3b9f420832799c57e91 in cordova-android's branch refs/heads/master from [~dpogue] [ https://gitbox.apache.org/repos/asf?p=cordova-android.git;h=e58453d ] Merge pull request #465 from erisu/CB-14201 CB-14201: (android) Removes Gradle property in-line command arguments… > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16671109#comment-16671109 ] ASF subversion and git services commented on CB-14201: -- Commit e58453d3e6f16e45fb9ac3b9f420832799c57e91 in cordova-android's branch refs/heads/master from [~dpogue] [ https://gitbox.apache.org/repos/asf?p=cordova-android.git;h=e58453d ] Merge pull request #465 from erisu/CB-14201 CB-14201: (android) Removes Gradle property in-line command arguments… > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16605285#comment-16605285 ] ASF GitHub Bot commented on CB-14201: - codecov-io edited a comment on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404037636 # [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=h1) Report > Merging [#465](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/23b24491c3e2ad5721a2813ec08f80985eca5e64?src=pr=desc) will **increase** coverage by `0.22%`. > The diff coverage is `78.78%`. [](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) ```diff @@Coverage Diff @@ ## master #465 +/- ## == + Coverage 61.64% 61.87% +0.22% == Files 16 17 +1 Lines1945 1975 +30 Branches 363 366 +3 == + Hits 1199 1222 +23 - Misses746 753 +7 ``` | [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) | Coverage Δ | | |---|---|---| | [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `44.55% <100%> (-0.57%)` | :arrow_down: | | [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `41.59% <33.33%> (-0.08%)` | :arrow_down: | | [...lates/cordova/lib/config/GradlePropertiesParser.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jb25maWcvR3JhZGxlUHJvcGVydGllc1BhcnNlci5qcw==) | `82.75% <82.75%> (ø)` | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=footer). Last update [23b2449...dacb0e5](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16577117#comment-16577117 ] ASF GitHub Bot commented on CB-14201: - codecov-io edited a comment on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404037636 # [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=h1) Report > Merging [#465](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/ebbd91f87dfb088fb69dfbeb300753bf7d19a194?src=pr=desc) will **increase** coverage by `0.34%`. > The diff coverage is `82.75%`. [](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) ```diff @@Coverage Diff @@ ## master #465 +/- ## == + Coverage62.9% 63.25% +0.34% == Files 15 16 +1 Lines1650 1679 +29 Branches 308 311 +3 == + Hits 1038 1062 +24 - Misses612 617 +5 ``` | [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) | Coverage Δ | | |---|---|---| | [...lates/cordova/lib/config/GradlePropertiesParser.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jb25maWcvR3JhZGxlUHJvcGVydGllc1BhcnNlci5qcw==) | `82.75% <82.75%> (ø)` | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=footer). Last update [ebbd91f...5042dfb](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16540120#comment-16540120 ] ASF GitHub Bot commented on CB-14201: - erisu commented on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404177315 Partial, Yes, The hardcoded defaults were removed so they can now be overridden by the user from the `gradle.properties`. **Additional Note:** Overridden defaults are accepted, not reverted, but the user will see an info message emitted with the Cordova's recommended value. Yes, the old defaults are used for creating the file. Since the first step for a new project is `cordova platform add`, it will naturally run `prepare` right after. This will create the new `gradle.properties` file with the original defaults and values that were previously hard-coded. The parser can read other non-default properties but is only used for the original defaults. As for supplying a lot of other configs, it was already possible by creating this file manually since Gradle naturally detects and uses the properties file. This PR main goal was removing the three hardcoded command-line arguments so it won't be locked in. __Here are also some additional use cases:__ **Use Case 1:** For existing projects that are missing the `gradle.properties` file, it will be created during the `prepare` step. Either triggered by `cordova (prepare|run|build)`. **Use Case 2:** Existing projects with existing `gradle.properties` file will check and add missing defaults. __And lastly, possible drawbacks, but not serious:__ **1.** If the user deletes a default, it will be re-appended. In this case, the user would just need to negate its value or take the default value listed by Gradle's docs if they don't want it or modify it. **2.** (Possibly a Very Small Edge Case) For existing projects, with no `gradle.properties` file, if only running `cordova compile` without preparing, builds will not run with any defaults. During local testing, with no `gradle.properties` file builds continue to build successfully. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16540038#comment-16540038 ] ASF GitHub Bot commented on CB-14201: - janpio edited a comment on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404155186 Nice. Did I get this right: You removed the previous hardcoded default config values and created a `gradle.properties` parser that uses the old defaults to create the file if it is missing, but can also read and supply lots of other configs that weren't there before. Correct? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16540035#comment-16540035 ] ASF GitHub Bot commented on CB-14201: - janpio commented on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404155186 Did I get this right: You removed the previous harcoded default configs and created a `gradle.properties` parser that uses the old defaults to create the file if it is missing. Correct? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16539529#comment-16539529 ] ASF GitHub Bot commented on CB-14201: - codecov-io commented on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404037636 # [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=h1) Report > Merging [#465](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/b6e4598e7c35dba433a5309c3c633485585dc692?src=pr=desc) will **increase** coverage by `0.47%`. > The diff coverage is `82.75%`. [](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) ```diff @@Coverage Diff @@ ## master #465 +/- ## == + Coverage 60.56% 61.04% +0.47% == Files 17 18 +1 Lines1694 1720 +26 Branches 312 315 +3 == + Hits 1026 1050 +24 - Misses668 670 +2 ``` | [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) | Coverage Δ | | |---|---|---| | [...in/templates/cordova/lib/builders/GradleBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9HcmFkbGVCdWlsZGVyLmpz) | `21.15% <ø> (+0.39%)` | :arrow_up: | | [...lates/cordova/lib/config/GradlePropertiesParser.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jb25maWcvR3JhZGxlUHJvcGVydGllc1BhcnNlci5qcw==) | `82.75% <82.75%> (ø)` | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=footer). Last update [b6e4598...c00d64d](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16539530#comment-16539530 ] ASF GitHub Bot commented on CB-14201: - codecov-io edited a comment on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404037636 # [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=h1) Report > Merging [#465](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=desc) into [master](https://codecov.io/gh/apache/cordova-android/commit/b6e4598e7c35dba433a5309c3c633485585dc692?src=pr=desc) will **increase** coverage by `0.47%`. > The diff coverage is `82.75%`. [](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) ```diff @@Coverage Diff @@ ## master #465 +/- ## == + Coverage 60.56% 61.04% +0.47% == Files 17 18 +1 Lines1694 1720 +26 Branches 312 315 +3 == + Hits 1026 1050 +24 - Misses668 670 +2 ``` | [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=tree) | Coverage Δ | | |---|---|---| | [...in/templates/cordova/lib/builders/GradleBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9HcmFkbGVCdWlsZGVyLmpz) | `21.15% <ø> (+0.39%)` | :arrow_up: | | [...lates/cordova/lib/config/GradlePropertiesParser.js](https://codecov.io/gh/apache/cordova-android/pull/465/diff?src=pr=tree#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jb25maWcvR3JhZGxlUHJvcGVydGllc1BhcnNlci5qcw==) | `82.75% <82.75%> (ø)` | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=footer). Last update [b6e4598...c00d64d](https://codecov.io/gh/apache/cordova-android/pull/465?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16539522#comment-16539522 ] ASF GitHub Bot commented on CB-14201: - erisu commented on issue #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465#issuecomment-404035534 @raphinesse I created this PR to replace the previous PR #459. As discusses in #459, it does feel better to remove the Gradle command line argument usage in-favor of the `gradle.properties` file. This also eliminates the idea or need for creating extra Cordova arguments for setting `jvmargs` or potential other Gradle property overrides. The user can instead change the values in gradle.properties. Please let me know what you think of this over the other PR. Also, this PR is linked to a new ticket, https://issues.apache.org/jira/browse/CB-14201, as the previous ticket only focused on `jvmargs`. This PR will affect all Gradle properties that we current set. (`org.gradle.daemon`, `org.gradle.jvmargs`, `android.useDeprecatedNdk`) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16539504#comment-16539504 ] ASF GitHub Bot commented on CB-14201: - erisu opened a new pull request #465: CB-14201: (android) Removes Gradle property in-line command arguments… URL: https://github.com/apache/cordova-android/pull/465 ### Platforms affected Android ### What does this PR do? This PR removes the Gradle properties in-line command arguments in favor of setting the gradle.properties file. This PR will give users more flexibility to override defaults that Cordova defines. ### What testing has been done on this change? - Locally ran the unit test, eslint, and code coverage. - Locally ran the Cordova prepare, run and build commands. - https://travis-ci.org/erisu/cordova-android/builds/402490984 - https://ci.appveyor.com/project/erisu/cordova-android/build/1.0.41 ### Checklist - [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database - [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB- is the JIRA ID & "android" is the platform affected. - [x] Added automated test coverage as appropriate for this change. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
[jira] [Commented] (CB-14201) Remove Gradle property in-line command arguments for gradle.properties
[ https://issues.apache.org/jira/browse/CB-14201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16539493#comment-16539493 ] Bryan Ellis commented on CB-14201: -- I will be submitting a PR for this. > Remove Gradle property in-line command arguments for gradle.properties > -- > > Key: CB-14201 > URL: https://issues.apache.org/jira/browse/CB-14201 > Project: Apache Cordova > Issue Type: Improvement > Components: cordova-android >Reporter: Bryan Ellis >Priority: Minor > > By passing in properties as an argument to the Gradle's command, it prevents > users from being able to override these values. > In one example, a user reported > ([CB-14139|https://issues.apache.org/jira/browse/CB-14139]) the need to be > able to set the jvmargs so builds works on 32bit Java. > Some other values Cordova passes by default are org.gradle.daemon and > android.useDeprecatedNdk. > By removing the command line argument usage and migrating to the usage of > gradle.properties, it gives users the flexibility to override defaults if > desired. > This ticket will also resolve > [CB-14139|https://issues.apache.org/jira/browse/CB-14139] but also remove the > other argument usage. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
