Skip to content
This repository has been archived by the owner on Oct 1, 2018. It is now read-only.

complicated and duplicated configs #161

Closed
pablomaurer opened this issue May 27, 2016 · 27 comments
Closed

complicated and duplicated configs #161

pablomaurer opened this issue May 27, 2016 · 27 comments
Milestone

Comments

@pablomaurer
Copy link
Contributor

pablomaurer commented May 27, 2016

Not a bug but just a thread to discuss about making the configuration maybe a bit easier. Above I listed all configuration possibilities with some thoughts on it. Maybe there are reasons behind it i just don't know, cause i didn't look at the code behind it.

project/www/chcp.json

build via build command

{
  "name": "app-name",
  "ios_identifier": "com.company.client1",
  "android_identifier": "whatever",
  "update": "resume",
  "content_url": "https://server.com/client1/www",
  "release": "2016.05.26-16.47.30"
}

project/cordova-hcp.json

contains everything the chcp contains..except the realease. maybe remove this file. And the build command doesn't copy the content from cordoca-hcp.json to chcp.json but just adds or replaces the release property of chcp.json

  "name": "app-name",
  "ios_identifier": "com.company.client1",
  "android_identifier": "whatever",
  "update": "resume",
  "content_url": "https://server.com/client1/www",

project/config.xml

Isn't the <config-file url="" /> ever the content_url+chcp.json from chcp.json ? Is there a reason some stuff is in config.xml and some stuff is in chcp.json?

<chcp>
  <config-file url="http://mynewdomain.ch/some/path/mobile/chcp.json"/>
  <local-development enabled="false"/>
  <auto-download enabled="false"/>
  <auto-install enabled="false"/>
</chcp>

api/chcp.configure (deprecated, new pass it as option in fetchUpdate())

  configurePlugin: function() {
    var options = {
      'config-file': 'https://mynewdomain.com/some/path/mobile/chcp.json'
    };
    chcp.configure(options, configureCallback);
  }

project/chcpbuild.options

Just to have all files listed

{
  "dev": {
    "config-file": "https://dev.company_server.com/mobile/www/chcp.json"
  },
  "production": {
    "config-file": "https://company_server.com/mobile/www/chcp.json"
  },
  "QA": {
    "config-file": "https://test.company_server.com/mobile/www/chcp.json"
  }
}

project/.chcpignroe

Just to have all files listed

project/.chcpenv

Just to have all files listed

@nikDemyankov
Copy link
Member

You are correct - there are lots of configs out there in the plugin. And maybe some simplification can be done. But in general - they are all quite simple. Will try to answer your questions below.

cordova-hcp.json

cordova-hcp.json is a base config for chcp.json. When cordova-hcp build is executed - CLI client takes preferences from cordova-hcp.json and adds them to www/chcp.json. You use cordova-hcp.json to define, for example, content_url, so you would not have to change it manually in generated file. Maybe it should be renamed to chcp.json to be more clear. But it should be there. Otherwise you will have to set content_url manually after each build.

config.xml

<config-file url="http://domain.com/www/chcp.json" /> is an entry point for the plugin. It uses this to download config from the server.

The idea is that you can place chcp.json and your content in different locations. Even on different servers, if you wish to. So, statement content_url+chcp.json == config-file is not really correct. In some cases it might be, but in general - might be too limited.

chcp.configure

This method will be deprecated and, eventually, removed. At the moment it is used, if you want to set config-file from JS side instead of config.xml; or you need to rewrite it. In next version you will be able to define config-file as an option for fetchUpdate method like this:

var options = {
  'config-file': 'http://myserver.com/chcp.json'
};
chcp.fetchUpdate(function(error, data) {
}, options);

So you would not need this configure method. It is already true for iOS (in the repo version), Android is in progress.

@pablomaurer
Copy link
Contributor Author

pablomaurer commented Jun 2, 2016

cordova-hcp.json

So the reason for this file is, that i don't have to set the content_url after each build manually. But I don't understand why I would have to set it, once set it's there forever. Maybe you mean this, because you are replacing the file chcp.json with the build command, but you could change it, that it does not replace the file and instead only update the release timestamp.

config.xml

Ok, but then the <config-file url="http://domain.com/www/chcp.json" /> should be optional and if it's not set then use content_url+chcp.json == config-file.

other thoughts

Wouldn't it be possible to move everything in the config.xml into the cordova-hcp.json/chcp.json?

Thanks ;)

Again thank you so much for this plugin - never thought I will find one which works so great like this!

@nikDemyankov
Copy link
Member

Maybe you mean this, because you are replacing the file chcp.json with the build command, but you could change it, that it does not replace the file and instead only update the release timestamp.

Yeah, probably it should work. One of the problems right now is that people don't understand why it is changing and how to make it constant.

Ok, but then the should be optional and if it's not set then use content_url+chcp.json == config-file.

Yes, that can be implemented.

Wouldn't it be possible to move everything in the config.xml into the cordova-hcp.json/chcp.json?

Yes, I think that's possible. And should also solve problem with Phonegap Build Service removing custom xml tags from config.xml.

Thanks for the ideas!

@robario
Copy link

robario commented Jun 9, 2016

@mnewmedia
One more thing. There is chcpbuild.options to override config.xml.
And I'm considering adding default option into it.
Could you please check my suggestion at #165 and #166?

@kartsims
Copy link

kartsims commented Jun 29, 2016

Thanks @mnewmedia for the inventory of the configuration files and pointing out this issue.

My input on this :

  • Totally agree with moving all config from config.xml to chcp.json. If there is no technical limitation, chcp.json should be the one and only place to set up CHCP
  • Not sure whether to keep cordova-hcp.json and generate www/chcp.json automatically via cordova-hcp build (replace the file so cordova-hcp.json is considered as the reference) OR remove cordova-hcp.json but sure one of them has to go...
    Another solution would be to pass the original file as a parameter to the build command that would replace www/chcp.json. This way if no parameter, you just update www/chcp.json, and if a file is passed as a parameter, you just take it, update release value and replace www/chcp,json
  • chcp.configure has to go... there are other ways to create/update JSON files from script
  • .chcpenv should not this be removed on build ? Caused weird behavior as reported in Error during update: The operation couldn’t be completed. (HCPPluginError error -1.) #174 and Error on update #179
  • chcp.options these could be merged in chcp.json even if they are not used. This was if we have an "original" chcp.json as mentioned before, the original could have all the available options and we could build www/chcp.json with such command
cordova-hcp build cordova-hcp.json --env=dev

This would replace the content_url according to the options.

I could help with the implementation of the new features if needed.

@nikDemyankov
Copy link
Member

nikDemyankov commented Jun 29, 2016

@kartsims thanks for the input and suggestion :) I think I'll start with configs simplification myself, and then everyone can help to check that I didn't mess up with that :) And if I did - add PR's with fixes/improvements.

Also, have a question to all, since we are talking about simplification. There are two additional preferences in chcp.json: android_identifier and ios_identifier (https://github.com/nordnet/cordova-hot-code-push/wiki/Application-config); plus a method in JS API - chcp.requestApplicationUpdate(). It was added in the very first version, but I feel like no one is using it and it should not be the part of the plugin. Plugin is responsible for updating the app, and if min_native_interface version is higher then the one in config.xml - it should just send the error to the web page to manage it. And if web page want's to redirect user to the Google Play/App Store - it can do it easily without the help from the plugin.

So I would like to remove that preferences as well, plus the JS method.

@nikDemyankov nikDemyankov added this to the v1.5.0 milestone Jun 29, 2016
@kartsims
Copy link

Yes I agree, it is not directly related to "hot code pushing" so it could be deprecated...

@nikDemyankov
Copy link
Member

nikDemyankov commented Jul 6, 2016

Okey, let's wrap it up a bit. I'll describe the changes that should be done in the plugin. Please, check, that I didn't miss anything. Any improvement suggestions are welcomed.

What needs to be removed

  1. Preferences from config.xml. They will be moved into chcp.json.
  2. cordova-hcp.json. No need for this config, we will have only www/chcp.json. And when cordova-hcp build is executed - it will replace release version in www/chcp.json file instead of regenerating it.
  3. Remove android_identifier and ios_identifier preferences from the config, since they are not used and should not be the part of the plugin.
  4. Remove chcp.requestApplicationUpdate() JS method. Plugin should be responsible only for updates.
  5. Remove chcp.configure() JS method. Parameters should be passed to the chcp.fetchUpdate() and chcp.installUpdate() methods directly.
  6. <local-development enabled="true | false"> preference will be removed. If you installed local development plugin - it's enabled. If you want to disable it - just delete the plugin.
  7. ASSETS_FOLDER_IN_NOT_YET_INSTALLED error should be removed. Instead, plugin will save your request internally and execute it when assets are installed on the external storage.

Plugin configs

  1. chcp.json - as main plugin config. But it's format will be changed a bit (see below). Required.
  2. chcp.manifest - as file's config. Required.
  3. chcpbuild.options - should stay as it is, maybe with some minor changes in the format. I don't think it's should be moved to chcp.json since it's a build options, not a plugin config. And it can be easily omitted by developer and replaced with similar npm scripts.

CLI client configs

  1. .chcpenv - it will be generated by the cordova-hcp server command automatically, but it will be used only by the local development plugin. And it will warn you about that. We need it for dev plugin, so it would set content url for you. But it's not gonna do anything, if cordova-hcp server is not running.
  2. .chcpignore - will remain as ignore list for cordova-hcp build/deploy.
  3. .chcpdeploy - holds preferences for deploy command. Currently for this purpose 2 configs are used: cordova-hcp.json and .chcplogin. Since we are removing cordova-hcp.json - preferences for deployment will be merged into one config, which will be created only if you use deploy function of the CLI client.

chcp.json format

The simplest version of the chcp.json file will be:

{
  "release": "2016.07.05-16.16.51",
  "content": "https://yourserver.com"
}

where:

  • release - current release version.
  • content - old content_url.

As you can see - there is no config-file preference. By default, plugin will assume that it's located in the root of the content url: https://yourserver.com/chcp.json.

The extended version will be like this:

{
  "release": {
    "version": "2016.07.05-16.16.51",
    "compare": "!= | >",
    "min_native_interface": "2"
  },
  "content": {
    "dir": "relative/path | http://content.server.com",
    "config": "https://config.server.com/chcp.json"
  },
  "update_auto_download": {
    "enabled": "true | false",
    "phase": "onstart | onresume"
  },
  "update_auto_install": {
    "enabled": "true | false",
    "phase": "onstart | onresume| ondownload"
  }
}
release block

release preference can be set to an object instead of just a string. Object has the following properties:

  • version - release version.
  • compare - as requested in here - it will allow you to define, how release version should be compared. By default it's !=, but you can tell the plugin to download only the newest releases by setting >.
  • min_native_interface - as before, responsible for telling the plugin, if it can install given release. I'm thinking to put it in here, since it's related to the release and it's version.
content block

content can be set to the object. In that case you can define content url and config-file url:

  • config - chcp.json url on your server.
  • dir - path to the content. Can be either full url, or a relative path to the chcp.json location (as requested here).
update_auto_download

By default, plugin will fetch updates from the server when application is launched (onstart event). But you can control this like that:

  • enabled - set to true, if you want plugin automatically download updates from the server; or false - to disable that.
  • phase - when download should be performed. Possible values are onstart (when app is launched), onresume (when app is resumed from the background state).
update_auto_install

By default, plugin will install updates when application is launched (onstart event). But you can control this like that:

  • enabled - set to true, if you want plugin automatically install updates; or false - to disable that.
  • phase - when installation should be performed. Possible values are onstart (when app is launched), onresume (when app is resumed from the background state), ondownload (when update was downloaded).

Result

With theses changes configuration should be a bit simpler and give you more space for maneuver. Also, since the changes are quite big and not backwards compatible - I'm thinking increasing plugin's version to 2.0.0 instead of planned 1.5.0.

Also, besides just changing the configs - I'll try to add more features to it (like checkForUpdate JS method), but will see. Before adding any new features need to simplify the configs :)

Anyway, please, read the above and tell me what you think and what needs to be improved.

@kartsims
Copy link

kartsims commented Jul 6, 2016

Thanks for the summary.

Sounds good to me, and agree with the major version bump.

The way I use it though I install/check for updates on resume but that's just me, I am not sure if this should be the default or not.

Great work, thanks !

@nikDemyankov
Copy link
Member

@kartsims thanks :)

Waiting for others to comment as well (if they want, of course :) ). Either way, will jump on the task from tomorrow.

@pablomaurer
Copy link
Contributor Author

Sounds perfect but i'm also for defaulting to resume, although it's not really important, since everyone can change it.

@robario
Copy link

robario commented Jul 7, 2016

Which file should we add it under VCS(e.g. Git)?

@nikDemyankov
Copy link
Member

nikDemyankov commented Jul 7, 2016

@robario www/chcp.json

update:

and www/chcp.manifest

@robario
Copy link

robario commented Jul 7, 2016

The release.version will be changed every cordova-hcp build/deploy, doesn't it?
I prefer that the VCS not contains automatic generated things. Just my opinion.

@nikDemyankov
Copy link
Member

Suggestions?

@robario
Copy link

robario commented Jul 7, 2016

I have no idea. Sorry.

@kartsims
Copy link

kartsims commented Jul 7, 2016

I don't think this is an issue because you are supposed to do cordova-hcp build so you don't even have to set it in your VCS, or you can set a dummy value, it doesn't matter

@nikDemyankov
Copy link
Member

I'd say: let's start with this approach, and then will see. If needed - adjustments will be made during the road.

@robario
Copy link

robario commented Jul 8, 2016

I have a thing to be worried about.
I think that release.version is affected by the timezone.
Is it better to use the UNIX epoch?

@nikDemyankov
Copy link
Member

Well, if you are building project on your work machine/build server - timezone is not an issue. You can, actually, put anything you want into release.version. But yeah, let's make it a unix epoch by default: maybe visually not that great, but easier to compare.

@robario
Copy link

robario commented Jul 8, 2016

Yes. The problem occurs only when specify release.compare.
Our developers who releases code are not alone and they are in the world wide.

Another solution is including timezone into release.version or force using UTC.
I prefer such as 2016-07-05T16:16:51-08:00(RFC 3339) that is parsable in JavaScript's Date
or 2016-07-02T00:16:51.000Z like ISO-8601 format must UTC by JavaScript's JSON.

@nikDemyankov
Copy link
Member

Thanks for the input. I think I'll use UNIX epoch for now, since it's simpler. And then in the future we can add some preference to cordova-hcp on what to use as release.version.

@nikDemyankov
Copy link
Member

Updated #161 (comment): added .chcpdeploy config description for CLI client. Used and created for deploy command only.

@Manduro
Copy link
Contributor

Manduro commented Sep 6, 2016

I would love it if the chcpbuild.options would be used by cordova-hcp build to replace the content property.

@nikDemyankov
Copy link
Member

@Manduro added it to the feature list in the CLI client. Thanks for the idea :)

@kwv
Copy link

kwv commented Mar 2, 2017

Not sure whether to keep cordova-hcp.json and generate www/chcp.json automatically via cordova-hcp build (replace the file so cordova-hcp.json is considered as the reference

I actually prefer this (assuming the filenames are consistent) as the www directory is frequently added to gitignore:
ionic 2 conference app


I am confused on the proposal for the the content: {'dir':'..'} object

config - chcp.json url on your server.
dir - path to the content. Can be either full url, or a relative path to the chcp.json location (as requested here).

Are you suggesting it is path relative to chcp.json not relative path to chcp.json ?

given:
https://webserver/chcp/chcp.json
https://webserver/appContextRoot/index.html

the content block would would look like

'content': {'config':'https://webserver/chcp/chcp.json', 'dir': '../appContextRoot/'}

@nordnet-deprecation-bot
Copy link
Contributor

👋 Hi! Thank you for your interest in this repo.

😢 We are not using nordnet/cordova-hot-code-push anymore, and we lack the manpower and the experience needed to maintain it. We are aware of the inconveniece that this may cause you. Feel free to use it as is, or create your own fork.

🔒 This will now be closed & locked.

ℹ️ Please see #371 for more information.

@nordnet nordnet locked and limited conversation to collaborators Sep 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants