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

Pass project config.xml path to platform's prepare #751

Merged
merged 1 commit into from
Mar 15, 2019

Conversation

dpogue
Copy link
Member

@dpogue dpogue commented Mar 15, 2019

Platforms affected

all

Motivation and Context

Currently we're passing in an already-constructed ConfigParser instance as part of the fake projectInfo object, but the problem is that it's constructed with the version of cordova-common depended on by cordova-lib as opposed to the version of cordova-common depended on by the platform.

See apache/cordova-ios#535 and apache/cordova-android#690

Description

Pass the path to the root config.xml in, so that we can construct the ConfigParser at the Platform API level, using the correct version.

In the meantime, for compatibility reasons, we have to keep passing in a ConfigParser instance, and the Platform API needs to handle the case where rootConfigXml is undefined and it needs to fetch the path out of the existing ConfigParser instance and then overwrite it with a newly constructed ConfigParser.

We should at some point look into making this "root project info" object a class in cordova-common so that we can document what it's expected to contain and rely on that both here and in the platforms.

Testing

Unit tests pass.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change

Currently we're passing in an already-constructed ConfigParser instance
as part of the fake projectInfo object, but the problem is that it's
constructed with the version of cordova-common depended on by
*cordova-lib* as opposed to the version of cordova-common depended on by
the *platform*. By passing the path in, we can construct the
ConfigParser at the Platform API level, using the correct version.

In the meantime, for compatibility reasons, we have to keep passing in a
ConfigParser instance, and the Platform API needs to handle the case
where rootConfigXml is undefined and it needs to fetch the path out of
the existing ConfigParser instance and then overwrite it with a newly
constructed ConfigParser.

We should at some point look into making this "root project info" object
a class in cordova-common so that we can document what it's expected to
contain and rely on that both here and in the platforms.
@dpogue dpogue requested a review from erisu March 15, 2019 05:45
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@erisu erisu merged commit b5edb57 into apache:master Mar 15, 2019
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

Successfully merging this pull request may close these issues.

2 participants