nordnet / cordova-hot-code-push

[DEPRECATED] - This plugin provides functionality to perform automatic updates of the web based content in your application.
https://github.com/nordnet/cordova-hot-code-push/issues/371
MIT License
978 stars 465 forks source link

complicated and duplicated configs #161

Closed pablomaurer closed 5 years ago

pablomaurer commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 7 years ago

@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 commented 7 years ago

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

My input on this :

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 commented 7 years ago

@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.

kartsims commented 7 years ago

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

nikDemyankov commented 7 years ago

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:

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:

content block

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

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:

update_auto_install

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

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 commented 7 years ago

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 commented 7 years ago

@kartsims thanks :)

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

pablomaurer commented 7 years ago

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

robario commented 7 years ago

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

nikDemyankov commented 7 years ago

@robario www/chcp.json

update:

and www/chcp.manifest

robario commented 7 years ago

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 commented 7 years ago

Suggestions?

robario commented 7 years ago

I have no idea. Sorry.

kartsims commented 7 years ago

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 commented 7 years ago

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

robario commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Updated https://github.com/nordnet/cordova-hot-code-push/issues/161#issuecomment-230724698: added .chcpdeploy config description for CLI client. Used and created for deploy command only.

Manduro commented 7 years ago

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

nikDemyankov commented 7 years ago

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

kwv commented 7 years ago

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 commented 5 years ago

👋 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.