Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cordova-create] New logic for setting attributes in package.json & config.xml (§3) #100

Closed
raphinesse opened this issue Jul 5, 2018 · 2 comments

Comments

@raphinesse
Copy link

I'm talking about the logic behind setting the appropriate fields for App ID,
App Name and App Version in the files package.json and config.xml.

Current situation

The most common strategy is to set the attribute in both files iff a value for
it was given to cordovaCreate. Exceptions are

  • App ID in package.json: set attribute if value given else set to helloworld
  • App Version in both files: always set to 1.0.0

Pain Points

  • Strategies employed for setting the attributes differ
  • Falling back to hard coded defaults is not very flexible (e.g. I usually start my version numbering at 0.1.0)

Proposal

For every file f and every attribute attr, do the following

if (attr in opts) {
    // Save attribute value passed to cordovaCreate to f
    f[attr] = opts[attr]
} else if (attr in f) {
    // Attribute already present in f and no override specified
    // => Leave the existing value untouched
} else if (isRequired(attr, f)) {
    handleMissingRequiredAttribute(attr, f)
}

where isRequired would have to be defined adequately
and handleMissingRequiredAttribute could either:

  1. Throw an error (my preference)
  2. Save some hard coded fallback value to f (possibly warn that f is invalid)

Related issues

CB-12274 - widget version number not copied from template config.xml file


Migrated from #89

@dpogue
Copy link
Member

dpogue commented Jul 17, 2018

If we're wanting to read/write name and version from package.json, I can add getters/setters for those in my proposed PackageHelper in cordova-common.

@raphinesse
Copy link
Author

This issue was moved to apache/cordova-create#22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants