You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by jbavari <gi...@git.apache.org> on 2014/04/08 21:53:15 UTC
[GitHub] cordova-plugman pull request: CB-6414 - fixes the issue where two ...
GitHub user jbavari opened a pull request:
https://github.com/apache/cordova-plugman/pull/72
CB-6414 - fixes the issue where two config.xml munges exists, it will st...
https://issues.apache.org/jira/browse/CB-6414
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbavari/cordova-plugman master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugman/pull/72.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #72
----
commit 067db2db5cdd2a309d83e04769a888b37d0cb36d
Author: Josh Bavari <jo...@raisemore.com>
Date: 2014-04-08T19:48:52Z
CB-6414 - fixes the issue where two config.xml munges exists, it will still write the correct config.xml output
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6414 - fixes the issue where two ...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on the pull request:
https://github.com/apache/cordova-plugman/pull/72#issuecomment-40000544
The best thing would be to avoid the double naming altogether when the munge is generated, so that it's always config.xml or res/xml/config.xml. If that's possible.
I'll look into the config_keeper tomorrow. @jbavari, tell me if you've already solved it by then to avoid duplicate work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6414 - fixes the issue where two ...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on the pull request:
https://github.com/apache/cordova-plugman/pull/72#issuecomment-39915727
Looks like the root of the problem is in config_keeper. The files there are identified by the "fake path" [see note], so when the same file is referenced as "config.xml" and "res/xml/config.xml", config keeper considers them to be two different files and changes for one of them get overwritten by the changes to the other.
I think it would be better to solve the problem in config_keeper rather than here, otherwise we might hit it again later with another file that might get referenced as two different names.
Maybe we could even go a level higher and avoid referring to the same file by two different names when generating the munge.
Anyway, if you decide to go with the current solution, please add a commend about what it does. Otherwise it is very cryptic.
[note]
Fake path is constructed as project_root/file_name. It is used because deducing the full real path requires globbing which is very slow (tens of milliseconds in some cases). The assumption was that the same file will alway be referred to with the same file_name in the munges. This assumption is broken in this case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6414 - fixes the issue where two ...
Posted by jbavari <gi...@git.apache.org>.
Github user jbavari commented on the pull request:
https://github.com/apache/cordova-plugman/pull/72#issuecomment-39987478
I will close it @agrieve
I have started looking into how the config_keeper is being used. I will try to figure it out still, however, I might not be as fast as @kamrik would be.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6414 - fixes the issue where two ...
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on the pull request:
https://github.com/apache/cordova-plugman/pull/72#issuecomment-39983493
I did merge this in, but forgot to close it (@Josh, can you close it?).
Mark - sounds like that's a better approach. Could you make the change?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6414 - fixes the issue where two ...
Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on the pull request:
https://github.com/apache/cordova-plugman/pull/72#issuecomment-40002842
@jbavari
Unrelated to this thread, you should add your name to github so readers of this thread can resolve jbavari == Josh Bavari, I was initially confused by @agrieve 's callout to @josh ( which is someone else ... )
This also applies to @agrieve ...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6414 - fixes the issue where two ...
Posted by jbavari <gi...@git.apache.org>.
Github user jbavari closed the pull request at:
https://github.com/apache/cordova-plugman/pull/72
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---