You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2021/04/18 01:47:32 UTC
[GitHub] [cordova-android] breautek opened a new pull request #1212: breaking: fixes target sdk override
breautek opened a new pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
Android
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
Fixes https://github.com/apache/cordova-android/issues/1178
Depends on https://github.com/apache/cordova-android/pull/1211
The original target SDK [fix PR](https://github.com/apache/cordova-android/issues/846) didn't actually solve problem. This is because the framework and the compile SDK did not change based on the `android-targetSdkVersion` preference, resulting the default API to be downloaded and used, despite the target SDK change.
### Description
<!-- Describe your changes in detail -->
This is a breaking change because this PR replaces the `cdkTargetSdkVersion` and `cdvCompileSdkVersion` with a new variable, `cdvSdkVersion`, which the `android-targetSdkVersion` sets.
`cdvSdkVersion` will control the android `target` API, as well as the `target SDK` and `compile SDK` settings of the android project.
### Testing
<!-- Please describe in detail how you tested your changes. -->
npm test & manual testing. If you remove all platform APIs, and set `android-targetSdkVersion` to 29 you should see:
- upon platform add, see android 29 printed
- upon cordova build, see API 29 and **only** API 29 tools downloaded.
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [x] I added automated test coverage as appropriate for this change
- [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [x] I've updated the documentation if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663524709
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
Review comment:
```js
// replace default with `required` or `optional`
if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
// Replace this with a `switch` statement after more use cases appear.
// `Number` could also be replaced with `mapping.type` since there is only one case and is Number.
if (configXmlValue || typeof configXmlValue === Number) {
mergedConfigs[mapping.gradleKey] = parseFloat(value);
} else if (Object.prototype.hasOwnProperty.call(mergedConfigs, mapping.gradleKey)) {
// Since configXmlValue had an incorrect data type, if the key already exists, remove it.
delete mergedConfigs[mapping.gradleKey];
}
} else {
// ..
}
```
Not tested, but maybe this will work instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cf9825c) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **decrease** coverage by `0.74%`.
> The diff coverage is `47.50%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 71.90% 71.15% -0.75%
==========================================
Files 21 21
Lines 1694 1709 +15
==========================================
- Hits 1218 1216 -2
- Misses 476 493 +17
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `44.30% <30.76%> (-3.48%)` | :arrow_down: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...cf9825c](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4b7ac21) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **increase** coverage by `0.29%`.
> The diff coverage is `88.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 71.90% 72.19% +0.29%
==========================================
Files 21 21
Lines 1694 1719 +25
==========================================
+ Hits 1218 1241 +23
- Misses 476 478 +2
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `50.44% <91.66%> (+2.66%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...4b7ac21](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663524709
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
Review comment:
```js
if (configXmlValue || typeof configXmlValue === mapping.type) {
mergedConfigs[mapping.gradleKey] = parseFloat(value);
} else if (Object.prototype.hasOwnProperty.call(mergedConfigs, mapping.gradleKey)) {
// Since configXmlValue had an incorrect data type, if the key already exists, remove it.
delete mergedConfigs[mapping.gradleKey];
}
```
Not tested, but maybe this will work instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826120459
@erisu
I made one last commit, as testing the latest changes produced a fail build locally...
The change ensure the settings have proper types. For example gradle expects a number for the target SDK setting and will crash if it receives a string,
but it looks like `config.json` could produce these values as strings if overridden via preferences.
Let me know what you think about the latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826396179
> Cool! I think that's a better solution.
>
> We should probably remove the placeholder and add a comment to the properties file now?
Will do. I'll remove it completely to avoid confusion and add a generated file notice. Also just realised this isn't working... it's writing out an empty value for some reason... so I'm troubleshooting that right now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663651848
##########
File path: framework/cordova.gradle
##########
@@ -151,12 +150,44 @@ def doGetConfigPreference(name, defaultValue) {
return ret
}
+def doApplyCordovaConfigCustomization() {
+ // Apply user overide properties that comes from the "--gradleArg=-P" parameters
+ if (project.hasProperty('cdvMinSdkVersion')) {
+ cordovaConfig.MIN_SDK_VERSION = Integer.parseInt('' + cdvMinSdkVersion)
+ }
+ if (project.hasProperty('cdvSdkVersion')) {
+ cordovaConfig.SDK_VERSION = Integer.parseInt('' + cdvSdkVersion)
+ }
+ if (project.hasProperty('cdvMaxSdkVersion')) {
+ cordovaConfig.MAX_SDK_VERSION = Integer.parseInt('' + cdvMaxSdkVersion)
+ }
+ if (project.hasProperty('cdvBuildToolsVersion')) {
+ cordovaConfig.BUILD_TOOLS_VERSION = cdvBuildToolsVersion
+ }
+ if (project.hasProperty('cdvAndroidXAppCompatVersion')) {
+ cordovaConfig.ANDROIDX_APP_COMPAT_VERSION = cdvAndroidXAppCompatVersion
+ }
+
+ // Ensure the latest installed build tools is selected, with or without defined override
+ cordovaConfig.LATEST_INSTALLED_BUILD_TOOLS = doFindLatestInstalledBuildTools(
+ cordovaConfig.BUILD_TOOLS_VERSION
+ )
+}
+
// Properties exported here are visible to all plugins.
ext {
+ def jsonFile = new File("$rootDir/config.json")
+ def parsedJson = new groovy.json.JsonSlurper().parseText(jsonFile.text)
+
+ cordovaConfig = parsedJson
Review comment:
Was combined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
raphinesse commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r658042594
##########
File path: spec/unit/prepare.spec.js
##########
@@ -783,6 +783,20 @@ describe('prepare', () => {
Api = rewire('../../bin/templates/cordova/Api');
prepare = rewire('../../bin/templates/cordova/lib/prepare');
+ prepare.__set__('require.resolve', (file) => {
+ if (file === 'cordova-android/framework/defaults.json') {
+ return path.resolve('./framework/defaults.json');
+ }
+ return path.resolve(file);
+ });
+
+ const defaults = require('../../framework/defaults.json');
+
+ prepare.__set__('fs', {
+ readJSONSync: () => defaults,
Review comment:
All reads will return the defaults. Is this desired? Moreover, the same object instance will be returned to all tests, making them prone to influence each other.
##########
File path: spec/unit/prepare.spec.js
##########
@@ -783,6 +783,20 @@ describe('prepare', () => {
Api = rewire('../../bin/templates/cordova/Api');
prepare = rewire('../../bin/templates/cordova/lib/prepare');
+ prepare.__set__('require.resolve', (file) => {
+ if (file === 'cordova-android/framework/defaults.json') {
+ return path.resolve('./framework/defaults.json');
+ }
+ return path.resolve(file);
+ });
Review comment:
There's more than one thing wrong with this mock. Would be great if we would not have to do this at all. But for now that would mean that we have to copy the defaults to the project as well. Then again, do we really need to revert to the defaults each time we prepare?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27b5838) into [master](https://codecov.io/gh/apache/cordova-android/commit/a45804329ba7bd50ebec8634ad0ffceab8e7aab9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a458043) will **increase** coverage by `0.19%`.
> The diff coverage is `85.41%`.
> :exclamation: Current head 27b5838 differs from pull request most recent head 92b9ea1. Consider uploading reports for the commit 92b9ea1 to get more accurate results
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 72.00% 72.20% +0.19%
==========================================
Files 21 21
Lines 1697 1720 +23
==========================================
+ Hits 1222 1242 +20
- Misses 475 478 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `95.00% <100.00%> (+0.80%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a458043...92b9ea1](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (132c4ef) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **increase** coverage by `0.18%`.
> The diff coverage is `85.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 71.90% 72.08% +0.18%
==========================================
Files 21 21
Lines 1694 1716 +22
==========================================
+ Hits 1218 1237 +19
- Misses 476 479 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...132c4ef](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663476050
##########
File path: spec/unit/prepare.spec.js
##########
@@ -783,6 +783,20 @@ describe('prepare', () => {
Api = rewire('../../bin/templates/cordova/Api');
prepare = rewire('../../bin/templates/cordova/lib/prepare');
+ prepare.__set__('require.resolve', (file) => {
+ if (file === 'cordova-android/framework/defaults.json') {
+ return path.resolve('./framework/defaults.json');
+ }
+ return path.resolve(file);
+ });
+
+ const defaults = require('../../framework/defaults.json');
+
+ prepare.__set__('fs', {
+ readJSONSync: () => defaults,
Review comment:
We can remove the test and figure something else out in the future.
We need to push forward with the release since we have less then a month before Google's Playstore new hard requirements. We should get something released as soon as possible to get user feedback on bugs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r664045272
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set for the items that are missing in `config.xml`.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally I think in common it should return undefined.
I think we should check first of the value is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@b2d9d63`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
=========================================
Coverage ? 71.99%
=========================================
Files ? 22
Lines ? 1707
Branches ? 0
=========================================
Hits ? 1229
Misses ? 478
Partials ? 0
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (ø)` | |
| [...lates/cordova/lib/config/GradlePropertiesParser.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jb25maWcvR3JhZGxlUHJvcGVydGllc1BhcnNlci5qcw==) | `86.27% <ø> (ø)` | |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `48.61% <84.00%> (ø)` | |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.90% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/TemplateFile.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9UZW1wbGF0ZUZpbGUuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `70.71% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b2d9d63...6f6765c](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663452890
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
Review comment:
The only key that can be added and is not listed in the defaults is `MAX_SDK_VERSION`.
Defining any random key is not a part of the use case, I beleive.
All default keys and `MAX_SDK_VERSION` has a reference somewhere in the Gradle build process.
I remember this mergeing process was to ensure that if the user deleted one of the default keys by mistake or removed their previous overriding setting, it would reapply the original default keys & values to ensure that the build process wont fail. Only the `MAX_SDK_VERSION` key would remain deleted, if they decided to remove this key. We do not want to enforce a max SDK version by default.
Also I think the merging covers cases of upgrading the platform. If we introduce new keys, it will update their existing project config to add in the new keys. Not everyone deletes the platforms and even some people commit it into their repo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (afd8f48) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **decrease** coverage by `0.66%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 71.90% 71.23% -0.67%
==========================================
Files 21 21
Lines 1694 1707 +13
==========================================
- Hits 1218 1216 -2
- Misses 476 491 +15
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `44.58% <33.33%> (-3.21%)` | :arrow_down: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...afd8f48](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663656740
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
Review comment:
Changed the code like so: https://github.com/apache/cordova-android/pull/1212/commits/4b7ac21d3de28486fa3419383514a69e56b3e6ef
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
Review comment:
Code updated here: https://github.com/apache/cordova-android/pull/1212/commits/4b7ac21d3de28486fa3419383514a69e56b3e6ef
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r664045272
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally I think in common it should return undefined.
I think we should check first of the value is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set for the items that are missing in `config.xml`.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally I think in common it should return undefined.
I think we should check first of the value is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set for the items that are missing in `config.xml`.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally, I think common it should be returning undefined for items are not founded. But, that is a different, future, task.
In this case, I think we should first check if the `rawValue` is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set for the items that are missing in `config.xml`.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally, I think common it should be returning undefined for items are not founded. But, that is a different, future, task.
In this case, I think we should first check if the `rawValue` is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue. trim() !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826390498
@raphinesse I think I can do one better...
I agree, having the `TemplateFile` class is a bit wasteful now that it's barely used. Since the only now only used to build a properties file, we can even use [properties-parser](https://www.npmjs.com/package/properties-parser), which is a dependency that is already used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826120459
@erisu
I made one last commit, as testing the latest changes produced a fail build locally...
The change ensure the settings have proper types. For example gradle expects a number for the target SDK setting and will crash if it receives a string,
but it looks like `config.json` could produce these values as strings if overridden via preferences.
Let me know what you think about the latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (008c206) into [master](https://codecov.io/gh/apache/cordova-android/commit/a45804329ba7bd50ebec8634ad0ffceab8e7aab9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a458043) will **increase** coverage by `0.18%`.
> The diff coverage is `85.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 72.00% 72.19% +0.18%
==========================================
Files 21 21
Lines 1697 1719 +22
==========================================
+ Hits 1222 1241 +19
- Misses 475 478 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
| [bin/templates/cordova/lib/env/java.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9lbnYvamF2YS5qcw==) | `100.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a458043...008c206](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663452890
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
Review comment:
The only key that can be added and is not listed in the defaults is `MAX_SDK_VERSION`.
Defining any random key is not a part of the use case, I beleive.
All default keys and `MAX_SDK_VERSION` has a reference somewhere in the Gradle build process.
I remember this mergeing process was to ensure that if the user deleted one of the default keys by mistake or removed their previous overriding setting, it would reapply the original default key & value to ensure that the build process wont fail. Only the `MAX_SDK_VERSION` key would remain deleted, if they decided to remove this key, as we do not want to enforce a max SDK version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663524709
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
Review comment:
```js
// Replace `default` with either `required` or `optional`
if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
/**
* Replace this with a `switch` case statement when more cases appear.
* Replace `Number` with `mapping.type`?
* Since there is currently one case, `mapping.type` === `Number`.
*/
if (configXmlValue || typeof configXmlValue === Number) {
mergedConfigs[mapping.gradleKey] = parseFloat(value);
} else if (Object.prototype.hasOwnProperty.call(mergedConfigs, mapping.gradleKey)) {
// Since configXmlValue had an incorrect data type, if the key already exists, remove it.
delete mergedConfigs[mapping.gradleKey];
}
} else {
// ..
}
```
Not tested, but maybe this will work instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu merged pull request #1212: feat!: unify & fix gradle library/tooling overrides
Posted by GitBox <gi...@apache.org>.
erisu merged pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663475222
##########
File path: framework/cordova.gradle
##########
@@ -151,12 +150,44 @@ def doGetConfigPreference(name, defaultValue) {
return ret
}
+def doApplyCordovaConfigCustomization() {
+ // Apply user overide properties that comes from the "--gradleArg=-P" parameters
+ if (project.hasProperty('cdvMinSdkVersion')) {
+ cordovaConfig.MIN_SDK_VERSION = Integer.parseInt('' + cdvMinSdkVersion)
+ }
+ if (project.hasProperty('cdvSdkVersion')) {
+ cordovaConfig.SDK_VERSION = Integer.parseInt('' + cdvSdkVersion)
+ }
+ if (project.hasProperty('cdvMaxSdkVersion')) {
+ cordovaConfig.MAX_SDK_VERSION = Integer.parseInt('' + cdvMaxSdkVersion)
+ }
+ if (project.hasProperty('cdvBuildToolsVersion')) {
+ cordovaConfig.BUILD_TOOLS_VERSION = cdvBuildToolsVersion
+ }
+ if (project.hasProperty('cdvAndroidXAppCompatVersion')) {
+ cordovaConfig.ANDROIDX_APP_COMPAT_VERSION = cdvAndroidXAppCompatVersion
+ }
+
+ // Ensure the latest installed build tools is selected, with or without defined override
+ cordovaConfig.LATEST_INSTALLED_BUILD_TOOLS = doFindLatestInstalledBuildTools(
+ cordovaConfig.BUILD_TOOLS_VERSION
+ )
+}
+
// Properties exported here are visible to all plugins.
ext {
+ def jsonFile = new File("$rootDir/config.json")
+ def parsedJson = new groovy.json.JsonSlurper().parseText(jsonFile.text)
+
+ cordovaConfig = parsedJson
Review comment:
I dont why it is split but maybe it can be merged together.
I have sometimes seen unusual cases where Gradle doesnt work like how you would expect. I am not sure if this was one of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27b5838) into [master](https://codecov.io/gh/apache/cordova-android/commit/a45804329ba7bd50ebec8634ad0ffceab8e7aab9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a458043) will **increase** coverage by `0.19%`.
> The diff coverage is `85.41%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 72.00% 72.20% +0.19%
==========================================
Files 21 21
Lines 1697 1720 +23
==========================================
+ Hits 1222 1242 +20
- Misses 475 478 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `95.00% <100.00%> (+0.80%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a458043...27b5838](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c1fbbd3) into [master](https://codecov.io/gh/apache/cordova-android/commit/5e7be8e1d6f9863f78a3bd481a3d366cc4d0a804?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5e7be8e) will **increase** coverage by `0.20%`.
> The diff coverage is `85.41%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 71.97% 72.17% +0.20%
==========================================
Files 21 21
Lines 1695 1718 +23
==========================================
+ Hits 1220 1240 +20
- Misses 475 478 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.84% <87.87%> (+1.90%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `95.00% <100.00%> (+0.80%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5e7be8e...c1fbbd3](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@b2d9d63`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `85.41%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
=========================================
Coverage ? 72.17%
=========================================
Files ? 21
Lines ? 1718
Branches ? 0
=========================================
Hits ? 1240
Misses ? 478
Partials ? 0
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (ø)` | |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.84% <87.87%> (ø)` | |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `95.00% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b2d9d63...c834cb4](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826390772
> @raphinesse I think I can do one better...
>
> I agree, having the `TemplateFile` class is a bit wasteful now that it's barely used. Since the only now only used to build a properties file, we can even use [properties-parser](https://www.npmjs.com/package/properties-parser), which is a dependency that is already used.
Agreed, that's probably even better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] MikeDimmickMnetics commented on pull request #1212: feat!: unify & fix gradle library/tooling overrides
Posted by GitBox <gi...@apache.org>.
MikeDimmickMnetics commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-1057981419
It is a bad idea to unify these settings. Target SDK is a _runtime_ setting which controls the level of backward compatibility fixes that Android is applying to the app. Compile SDK controls which declarations are being used to compile the code. They're separate things. You can still use features newer from API levels newer than your 'target SDK' setting, while keeping backward-compatible behaviour for the older parts of your app (or libraries) that you haven't upgraded yet.
I realise this is a bit academic for apps on the Play Store, because Google require Target SDK = 30, from November 2021, even for updates to older apps. However, for anyone doing private distribution, including Managed Play Store, forcing an update to the target SDK in order to change the compile SDK is unnecessary. I'm not sure why you'd want to set the compile SDK lower than the target SDK, though.
My line-of-business app, using cordova-android 6.4, currently uses compile SDK 26, to pick up support for adaptive icons, but target SDK 23, because we've not fixed any compatibility issues later than Marshmallow. I don't want to have to suddenly fix Android 11 compat issues, such as scoped storage enforcement, when upgrading to cordova-android 10.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@b2d9d63`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `85.41%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
=========================================
Coverage ? 72.17%
=========================================
Files ? 21
Lines ? 1718
Branches ? 0
=========================================
Hits ? 1240
Misses ? 478
Partials ? 0
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (ø)` | |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.84% <87.87%> (ø)` | |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `95.00% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b2d9d63...feeee5d](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663475035
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
Review comment:
I think the naming for this could be changed.
Every option has a default value set in the `defaults.json`. This option on the other hand does not have a default value because we do not recommend users to set a max SDK version on their app. It is limiting.
I think the original use case was for a flag to handle the ability to see if the option can be cleaned out of the project gradle configs object.
If optional, and is not set, strip out the item from the object if present.
So the flag could be renamed to `optional` or `required`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
raphinesse commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r657956449
##########
File path: bin/lib/create.js
##########
@@ -247,13 +247,16 @@ exports.create = function (project_path, config, options, events) {
? config.name().replace(/[^\w.]/g, '_') : 'CordovaExample';
var safe_activity_name = config.android_activityName() || options.activityName || 'MainActivity';
- var target_api = check_reqs.get_target();
+ let target_api = parseInt(config.getPreference('android-targetSdkVersion', 'android'), 10);
+ if (isNaN(target_api)) {
+ target_api = constants.SDK_VERSION;
+ }
// Make the package conform to Java package types
return exports.validatePackageName(package_name)
.then(function () {
return exports.validateProjectName(project_name);
- }).then(function () {
+ }).then(async function () {
Review comment:
Reverting leftover `async`
```suggestion
}).then(function () {
```
##########
File path: bin/lib/create.js
##########
@@ -69,11 +66,14 @@ function copyJsAndLibrary (projectPath, shared, projectName, isLegacy) {
} else {
fs.ensureDirSync(nestedCordovaLibPath);
fs.copySync(path.join(ROOT, 'framework', 'AndroidManifest.xml'), path.join(nestedCordovaLibPath, 'AndroidManifest.xml'));
- fs.copySync(path.join(ROOT, 'framework', 'project.properties'), path.join(nestedCordovaLibPath, 'project.properties'));
+ const propertiesEditor = createEditor(path.join(ROOT, 'framework', 'project.properties'));
+ propertiesEditor.set('target', `android-${targetAPI || constants.SDK_VERSION}`);
+ propertiesEditor.save(path.join(nestedCordovaLibPath, 'project.properties'));
fs.copySync(path.join(ROOT, 'framework', 'build.gradle'), path.join(nestedCordovaLibPath, 'build.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'cordova.gradle'), path.join(nestedCordovaLibPath, 'cordova.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'repositories.gradle'), path.join(nestedCordovaLibPath, 'repositories.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'src'), path.join(nestedCordovaLibPath, 'src'));
+ fs.copySync(path.join(ROOT, 'framework', 'defaults.json'), path.join(projectPath, 'config.json'));
Review comment:
Assuming that the file `config.json` is something we invent in this PR, I would suggest we try to find a more specific name. From the top of my head: maybe something like `native-build-{config,settings,params}.json`?
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
Review comment:
We never use the value of `mapping.default`
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
+ mergedConfigs[mapping.gradleKey] = configXmlValue;
+ } else if (Object.prototype.hasOwnProperty.call(mergedConfigs, mapping.gradleKey)) {
+ delete mergedConfigs[mapping.gradleKey];
+ }
+ } else {
+ let value = configXmlValue || defaultGradleConfig[mapping.gradleKey];
+
+ switch (mapping.type) {
+ default:
+ case String:
+ value = value.toString();
+ break;
+ case Number:
+ value = parseFloat(value);
Review comment:
Since we are parsing only SDK versions, this should rather be `parseInt(value, 10)`, no?
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
Review comment:
The only mapping entry that has a default key is of type Number, so why handle boolean inputs?
##########
File path: framework/cordova.gradle
##########
@@ -151,12 +150,44 @@ def doGetConfigPreference(name, defaultValue) {
return ret
}
+def doApplyCordovaConfigCustomization() {
+ // Apply user overide properties that comes from the "--gradleArg=-P" parameters
+ if (project.hasProperty('cdvMinSdkVersion')) {
+ cordovaConfig.MIN_SDK_VERSION = Integer.parseInt('' + cdvMinSdkVersion)
+ }
+ if (project.hasProperty('cdvSdkVersion')) {
+ cordovaConfig.SDK_VERSION = Integer.parseInt('' + cdvSdkVersion)
+ }
+ if (project.hasProperty('cdvMaxSdkVersion')) {
+ cordovaConfig.MAX_SDK_VERSION = Integer.parseInt('' + cdvMaxSdkVersion)
+ }
+ if (project.hasProperty('cdvBuildToolsVersion')) {
+ cordovaConfig.BUILD_TOOLS_VERSION = cdvBuildToolsVersion
+ }
+ if (project.hasProperty('cdvAndroidXAppCompatVersion')) {
+ cordovaConfig.ANDROIDX_APP_COMPAT_VERSION = cdvAndroidXAppCompatVersion
+ }
+
+ // Ensure the latest installed build tools is selected, with or without defined override
+ cordovaConfig.LATEST_INSTALLED_BUILD_TOOLS = doFindLatestInstalledBuildTools(
+ cordovaConfig.BUILD_TOOLS_VERSION
+ )
+}
+
// Properties exported here are visible to all plugins.
ext {
+ def jsonFile = new File("$rootDir/config.json")
+ def parsedJson = new groovy.json.JsonSlurper().parseText(jsonFile.text)
+
+ cordovaConfig = parsedJson
Review comment:
Can't we just do the following?
```suggestion
cordovaConfig = new groovy.json.JsonSlurper().parseText(jsonFile.text)
```
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
Review comment:
Should this be `projectGradleConfig` instead of `profileGradleConfig`?
##########
File path: bin/templates/cordova/lib/builders/ProjectBuilder.js
##########
@@ -287,6 +288,16 @@ class ProjectBuilder {
});
}
+ /**
+ * @private
+ * @returns The user defined configs
+ */
+ _getCordovaConfig () {
+ return JSON.parse(fs.readFileSync(path.resolve(this.root, 'config.json'), {
+ encoding: 'utf8'
+ }));
Review comment:
Using fs-extra convenience function:
```suggestion
return fs.readJSONSync(path.join(this.root, 'config.json'));
```
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
Review comment:
Is `projectGradleConfig` allowed to define keys that are not in `defaultGradleConfig`? If not, then we can omit this and use the defaults straight away.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663476050
##########
File path: spec/unit/prepare.spec.js
##########
@@ -783,6 +783,20 @@ describe('prepare', () => {
Api = rewire('../../bin/templates/cordova/Api');
prepare = rewire('../../bin/templates/cordova/lib/prepare');
+ prepare.__set__('require.resolve', (file) => {
+ if (file === 'cordova-android/framework/defaults.json') {
+ return path.resolve('./framework/defaults.json');
+ }
+ return path.resolve(file);
+ });
+
+ const defaults = require('../../framework/defaults.json');
+
+ prepare.__set__('fs', {
+ readJSONSync: () => defaults,
Review comment:
We can remove the test and figure something else out in the future.
We need to push forward with the release since we have less then a month before Google's Playstore new hard requirements. We should get something released as soon as possible to get user feedback on bugs. Rather not have tests holding the release back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@b2d9d63`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `83.67%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
=========================================
Coverage ? 71.93%
=========================================
Files ? 22
Lines ? 1707
Branches ? 0
=========================================
Hits ? 1228
Misses ? 479
Partials ? 0
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (ø)` | |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `48.61% <84.00%> (ø)` | |
| [bin/templates/cordova/lib/TemplateFile.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9UZW1wbGF0ZUZpbGUuanM=) | `90.00% <90.00%> (ø)` | |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.90% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `70.71% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b2d9d63...26a411b](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826393940
Cool! I think that's a better solution.
We should probably remove the placeholder and add a comment to the properties file now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663475616
##########
File path: spec/unit/prepare.spec.js
##########
@@ -783,6 +783,20 @@ describe('prepare', () => {
Api = rewire('../../bin/templates/cordova/Api');
prepare = rewire('../../bin/templates/cordova/lib/prepare');
+ prepare.__set__('require.resolve', (file) => {
+ if (file === 'cordova-android/framework/defaults.json') {
+ return path.resolve('./framework/defaults.json');
+ }
+ return path.resolve(file);
+ });
Review comment:
I think it is good to revert each time we prepare. User could accidentally delete a key from the file that is needed for the builds to work. Then we would like repopulat with the default.
It is also another way for users to quickly remove a change value back to the default without needing to remember what was the exact default value, as it would re-add the defaults.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu merged pull request #1212: feat!: unify & fix gradle library/tooling overrides
Posted by GitBox <gi...@apache.org>.
erisu merged pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826120459
@erisu
I made one last commit, as testing the latest changes produced a fail build locally...
The change ensure the settings have proper types. For example gradle expects a number for the target SDK setting and will crash if it receives a string,
but it looks like `config.json` could produce these values as strings if overridden via preferences.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663656502
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
Review comment:
Dropped the need of checking for `mapping.default`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ea64965) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **increase** coverage by `0.18%`.
> The diff coverage is `85.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 71.90% 72.08% +0.18%
==========================================
Files 21 21
Lines 1694 1716 +22
==========================================
+ Hits 1218 1237 +19
- Misses 476 479 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...ea64965](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r620425378
##########
File path: bin/lib/create.js
##########
@@ -66,8 +66,10 @@ function copyJsAndLibrary (projectPath, shared, projectName, targetAPI) {
} else {
fs.ensureDirSync(nestedCordovaLibPath);
fs.copySync(path.join(ROOT, 'framework', 'AndroidManifest.xml'), path.join(nestedCordovaLibPath, 'AndroidManifest.xml'));
- TemplateFile.render(path.join(ROOT, 'framework', 'project.properties'), path.join(nestedCordovaLibPath, 'project.properties'), {
- SDK_VERSION: targetAPI || constants.SDK_VERSION
+ await new Promise((resolve) => {
+ const propertiesEditor = createEditor(path.join(ROOT, 'framework', 'project.properties'));
+ propertiesEditor.set('target', `android-${targetAPI || constants.SDK_VERSION}`);
+ propertiesEditor.save(path.join(nestedCordovaLibPath, 'project.properties'), resolve);
Review comment:
Interesting, I can revert that to avoid unnecessary breaking changes then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a16d183) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **decrease** coverage by `0.66%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 71.90% 71.23% -0.67%
==========================================
Files 21 21
Lines 1694 1707 +13
==========================================
- Hits 1218 1216 -2
- Misses 476 491 +15
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `44.58% <33.33%> (-3.21%)` | :arrow_down: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...a16d183](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r658047442
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
+ mergedConfigs[mapping.gradleKey] = configXmlValue;
+ } else if (Object.prototype.hasOwnProperty.call(mergedConfigs, mapping.gradleKey)) {
+ delete mergedConfigs[mapping.gradleKey];
+ }
+ } else {
+ let value = configXmlValue || defaultGradleConfig[mapping.gradleKey];
+
+ switch (mapping.type) {
+ default:
+ case String:
+ value = value.toString();
+ break;
+ case Number:
+ value = parseFloat(value);
Review comment:
I would rather avoid assuming that a setting value would be an integer, even if all the current number settings are integers right now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
raphinesse commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r620066627
##########
File path: bin/lib/create.js
##########
@@ -66,8 +66,10 @@ function copyJsAndLibrary (projectPath, shared, projectName, targetAPI) {
} else {
fs.ensureDirSync(nestedCordovaLibPath);
fs.copySync(path.join(ROOT, 'framework', 'AndroidManifest.xml'), path.join(nestedCordovaLibPath, 'AndroidManifest.xml'));
- TemplateFile.render(path.join(ROOT, 'framework', 'project.properties'), path.join(nestedCordovaLibPath, 'project.properties'), {
- SDK_VERSION: targetAPI || constants.SDK_VERSION
+ await new Promise((resolve) => {
+ const propertiesEditor = createEditor(path.join(ROOT, 'framework', 'project.properties'));
+ propertiesEditor.set('target', `android-${targetAPI || constants.SDK_VERSION}`);
+ propertiesEditor.save(path.join(nestedCordovaLibPath, 'project.properties'), resolve);
Review comment:
This code handles no errors. Fortunately, as it turns out, `properties-parser` [supports sync writing](https://github.com/xavi-/node-properties-parser/blob/fb1b7038380fa295ff80ed0d1a1fad3ad1788738/index.js#L367-L371) it is just missing from the documentation. We also already make use of that fact in `GradlePropertiesParser`. So we should probably just use the sync version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663517328
##########
File path: bin/lib/create.js
##########
@@ -69,11 +66,14 @@ function copyJsAndLibrary (projectPath, shared, projectName, isLegacy) {
} else {
fs.ensureDirSync(nestedCordovaLibPath);
fs.copySync(path.join(ROOT, 'framework', 'AndroidManifest.xml'), path.join(nestedCordovaLibPath, 'AndroidManifest.xml'));
- fs.copySync(path.join(ROOT, 'framework', 'project.properties'), path.join(nestedCordovaLibPath, 'project.properties'));
+ const propertiesEditor = createEditor(path.join(ROOT, 'framework', 'project.properties'));
+ propertiesEditor.set('target', `android-${targetAPI || constants.SDK_VERSION}`);
+ propertiesEditor.save(path.join(nestedCordovaLibPath, 'project.properties'));
fs.copySync(path.join(ROOT, 'framework', 'build.gradle'), path.join(nestedCordovaLibPath, 'build.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'cordova.gradle'), path.join(nestedCordovaLibPath, 'cordova.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'repositories.gradle'), path.join(nestedCordovaLibPath, 'repositories.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'src'), path.join(nestedCordovaLibPath, 'src'));
+ fs.copySync(path.join(ROOT, 'framework', 'defaults.json'), path.join(projectPath, 'config.json'));
Review comment:
Maybe change:
* `defaults.json` to `cdv-gradle-config-defaults.json`
* `config.json` to `cdv-gradle-config.json`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@b2d9d63`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `85.45%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
=========================================
Coverage ? 72.23%
=========================================
Files ? 22
Lines ? 1725
Branches ? 0
=========================================
Hits ? 1246
Misses ? 479
Partials ? 0
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (ø)` | |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.84% <87.87%> (ø)` | |
| [bin/templates/cordova/lib/TemplateFile.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9UZW1wbGF0ZUZpbGUuanM=) | `90.00% <90.00%> (ø)` | |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.90% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b2d9d63...e7d8cd9](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826120459
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r663517328
##########
File path: bin/lib/create.js
##########
@@ -69,11 +66,14 @@ function copyJsAndLibrary (projectPath, shared, projectName, isLegacy) {
} else {
fs.ensureDirSync(nestedCordovaLibPath);
fs.copySync(path.join(ROOT, 'framework', 'AndroidManifest.xml'), path.join(nestedCordovaLibPath, 'AndroidManifest.xml'));
- fs.copySync(path.join(ROOT, 'framework', 'project.properties'), path.join(nestedCordovaLibPath, 'project.properties'));
+ const propertiesEditor = createEditor(path.join(ROOT, 'framework', 'project.properties'));
+ propertiesEditor.set('target', `android-${targetAPI || constants.SDK_VERSION}`);
+ propertiesEditor.save(path.join(nestedCordovaLibPath, 'project.properties'));
fs.copySync(path.join(ROOT, 'framework', 'build.gradle'), path.join(nestedCordovaLibPath, 'build.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'cordova.gradle'), path.join(nestedCordovaLibPath, 'cordova.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'repositories.gradle'), path.join(nestedCordovaLibPath, 'repositories.gradle'));
fs.copySync(path.join(ROOT, 'framework', 'src'), path.join(nestedCordovaLibPath, 'src'));
+ fs.copySync(path.join(ROOT, 'framework', 'defaults.json'), path.join(projectPath, 'config.json'));
Review comment:
renamed the following:
* `defaults.json` to `cdv-gradle-config-defaults.json`
* `config.json` to `cdv-gradle-config.json`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (feedcd5) into [master](https://codecov.io/gh/apache/cordova-android/commit/a45804329ba7bd50ebec8634ad0ffceab8e7aab9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a458043) will **increase** coverage by `0.18%`.
> The diff coverage is `85.10%`.
> :exclamation: Current head feedcd5 differs from pull request most recent head 92b9ea1. Consider uploading reports for the commit 92b9ea1 to get more accurate results
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 72.00% 72.19% +0.18%
==========================================
Files 21 21
Lines 1697 1719 +22
==========================================
+ Hits 1222 1241 +19
- Misses 475 478 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a458043...92b9ea1](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r664045272
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set for the items that are missing in `config.xml`.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally, I think common it should be returning undefined for items are not founded. But, that is a different, future, task.
In this case, I think we should first check if the `rawValue` is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue. trim() !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
raphinesse commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826390772
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] breautek commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-826392293
Only thing about using `properties-parser` is that they don't have a sync API, so I had to turn the function into an asynchronous method -- but I believe the module is private anyway (exported purely for unit testing purposes).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c56aef6) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **increase** coverage by `0.18%`.
> The diff coverage is `85.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 71.90% 72.08% +0.18%
==========================================
Files 21 21
Lines 1694 1716 +22
==========================================
+ Hits 1218 1237 +19
- Misses 476 479 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...c56aef6](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r664045272
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally I think in common it should return undefined.
I think we should check first of the value is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (92b9ea1) into [master](https://codecov.io/gh/apache/cordova-android/commit/a45804329ba7bd50ebec8634ad0ffceab8e7aab9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a458043) will **increase** coverage by `0.18%`.
> The diff coverage is `85.10%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
+ Coverage 72.00% 72.19% +0.18%
==========================================
Files 21 21
Lines 1697 1719 +22
==========================================
+ Hits 1222 1241 +19
- Misses 475 478 +3
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `49.69% <87.87%> (+1.91%)` | :arrow_up: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a458043...92b9ea1](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (10e8718) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **decrease** coverage by `0.66%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 71.90% 71.23% -0.67%
==========================================
Files 21 21
Lines 1694 1707 +13
==========================================
- Hits 1218 1216 -2
- Misses 476 491 +15
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `44.58% <33.33%> (-3.21%)` | :arrow_down: |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.96% <100.00%> (+0.77%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...10e8718](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter edited a comment on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1212](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (171c420) into [master](https://codecov.io/gh/apache/cordova-android/commit/47aa116b1dbf538259ebbbc3465c575c2d9c673d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47aa116) will **decrease** coverage by `0.81%`.
> The diff coverage is `56.25%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 71.90% 71.08% -0.82%
==========================================
Files 21 22 +1
Lines 1694 1705 +11
==========================================
- Hits 1218 1212 -6
- Misses 476 493 +17
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (-0.98%)` | :arrow_down: |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `44.75% <36.00%> (-3.04%)` | :arrow_down: |
| [...in/templates/cordova/lib/gradle-config-defaults.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9ncmFkbGUtY29uZmlnLWRlZmF1bHRzLmpz) | `66.66% <66.66%> (ø)` | |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.90% <100.00%> (+0.71%)` | :arrow_up: |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `71.32% <100.00%> (-1.34%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [47aa116...171c420](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] raphinesse commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
raphinesse commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r658052143
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +76,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+function updateUserProjectGradleConfig (configXml, defaultGradleConfigPath, projectGradleConfigPath) {
+ const defaultGradleConfig = fs.readJSONSync(defaultGradleConfigPath);
+ const profileGradleConfig = fs.readJSONSync(projectGradleConfigPath);
+
+ // Replace modified configs with defaults
+ const mergedConfigs = Object.assign(profileGradleConfig, defaultGradleConfig);
+
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', default: null, type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ configXmlToGradleMapping.forEach(mapping => {
+ const configXmlValue = configXml.getPreference(mapping.xmlKey, 'android');
+
+ if (Object.prototype.hasOwnProperty.call(mapping, 'default')) {
+ if (configXmlValue || typeof configXmlValue === 'boolean') {
+ mergedConfigs[mapping.gradleKey] = configXmlValue;
+ } else if (Object.prototype.hasOwnProperty.call(mergedConfigs, mapping.gradleKey)) {
+ delete mergedConfigs[mapping.gradleKey];
+ }
+ } else {
+ let value = configXmlValue || defaultGradleConfig[mapping.gradleKey];
+
+ switch (mapping.type) {
+ default:
+ case String:
+ value = value.toString();
+ break;
+ case Number:
+ value = parseFloat(value);
Review comment:
Fair enough
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a change in pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#discussion_r664045272
##########
File path: bin/templates/cordova/lib/prepare.js
##########
@@ -92,6 +74,76 @@ module.exports.prepare = function (cordovaProject, options) {
});
};
+/** @param {PlatformApi} project */
+function updateUserProjectGradleConfig (project) {
+ // Generate project gradle config
+ const projectGradleConfig = {
+ ...require('cordova-android/framework/cdv-gradle-config-defaults.json'),
+ ...getUserGradleConfig(project._config)
+ };
+
+ // Write out changes
+ const projectGradleConfigPath = path.join(project.root, 'cdv-gradle-config.json');
+ fs.writeJSONSync(projectGradleConfigPath, projectGradleConfig, { spaces: 2 });
+}
+
+function getUserGradleConfig (configXml) {
+ const configXmlToGradleMapping = [
+ { xmlKey: 'android-minSdkVersion', gradleKey: 'MIN_SDK_VERSION', type: Number },
+ { xmlKey: 'android-maxSdkVersion', gradleKey: 'MAX_SDK_VERSION', type: Number },
+ { xmlKey: 'android-targetSdkVersion', gradleKey: 'SDK_VERSION', type: Number },
+ { xmlKey: 'android-buildToolsVersion', gradleKey: 'BUILD_TOOLS_VERSION', type: String },
+ { xmlKey: 'GradleVersion', gradleKey: 'GRADLE_VERSION', type: String },
+ { xmlKey: 'AndroidGradlePluginVersion', gradleKey: 'AGP_VERSION', type: String },
+ { xmlKey: 'GradlePluginKotlinVersion', gradleKey: 'KOTLIN_VERSION', type: String },
+ { xmlKey: 'AndroidXAppCompatVersion', gradleKey: 'ANDROIDX_APP_COMPAT_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesVersion', gradleKey: 'GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION', type: String },
+ { xmlKey: 'GradlePluginGoogleServicesEnabled', gradleKey: 'IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED', type: Boolean },
+ { xmlKey: 'GradlePluginKotlinEnabled', gradleKey: 'IS_GRADLE_PLUGIN_KOTLIN_ENABLED', type: Boolean }
+ ];
+
+ return configXmlToGradleMapping.reduce((config, mapping) => {
+ const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
+ const typecastValue = castValueToType(rawValue, mapping.type);
+
+ if (typecastValue !== undefined) {
+ config[mapping.gradleKey] = typecastValue;
+ }
+ return config;
+ }, {});
Review comment:
This code is causing the defaults to not be set for the items that are missing in `config.xml`.
This is probally because of how `cordova-common` returns an empty string when it does not find the preference with `getPreference`. Personally, I think common it should be returning undefined for items are not founded. But, that is a different, future, task.
In this case, I think we should first check if the `rawValue` is not an empty string.
```suggestion
return configXmlToGradleMapping.reduce((config, mapping) => {
const rawValue = configXml.getPreference(mapping.xmlKey, 'android');
if (rawValue !== '') {
const typecastValue = castValueToType(rawValue, mapping.type);
if (typecastValue !== undefined) {
config[mapping.gradleKey] = typecastValue;
}
}
return config;
}, {});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter commented on pull request #1212: breaking: fixes target sdk override
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1212:
URL: https://github.com/apache/cordova-android/pull/1212#issuecomment-823747226
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@b2d9d63`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1212 +/- ##
=========================================
Coverage ? 71.99%
=========================================
Files ? 22
Lines ? 1707
Branches ? 0
=========================================
Hits ? 1229
Misses ? 478
Partials ? 0
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/templates/cordova/lib/builders/ProjectBuilder.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9idWlsZGVycy9Qcm9qZWN0QnVpbGRlci5qcw==) | `73.85% <0.00%> (ø)` | |
| [...lates/cordova/lib/config/GradlePropertiesParser.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jb25maWcvR3JhZGxlUHJvcGVydGllc1BhcnNlci5qcw==) | `86.27% <ø> (ø)` | |
| [bin/templates/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9wcmVwYXJlLmpz) | `48.61% <84.00%> (ø)` | |
| [bin/lib/create.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL2xpYi9jcmVhdGUuanM=) | `94.90% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/TemplateFile.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9UZW1wbGF0ZUZpbGUuanM=) | `100.00% <100.00%> (ø)` | |
| [bin/templates/cordova/lib/check\_reqs.js](https://codecov.io/gh/apache/cordova-android/pull/1212/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmluL3RlbXBsYXRlcy9jb3Jkb3ZhL2xpYi9jaGVja19yZXFzLmpz) | `70.71% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b2d9d63...6f6765c](https://codecov.io/gh/apache/cordova-android/pull/1212?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org