read version information from... version.json

[Docteh]

Hi, version.json helpfully holds the gitChangesetId, I think it should include the version as well

• version is now written to version.json, value is the one in package.json

• using this version, instead of asking for a manifest via chrome.runtime.getManifest()

• eventpage.js still has its own getManifestVersion() that it uses.

• skip checking for updates when running as Other.

[mikeller]

Disagree here - the Chrome web app version shown in Chrome is the version in manifest.json. Setting the version from package.json and risking that the the version 'outside' and 'inside' the app differ is wrong.
Also, since Chrome web apps only support x.y.z, we should read version_name from package.json as well, to allow for RC versioning on the standalone app side.
In general I am not sure if this is the right move - at least for the Chrome web app we have to distribute manifest.json, so if we read the version out of there we cut out the potential for ambiguity.
Not sure if we can remove package.json from the NW.js distribution, but if we can then we should do so as part of this change.

[Docteh]

For nwjs the call to chrome.runtime.getManifest() reads in package.json, I thought it was reading in manifest.json.

Could we modify the gulp system to also revise the version in manifest.json as part of the dist process? Unlike the blackbox viewer the dist process is required to run as a Chrome web app anyways as some files need to be moved around. I guess then we need to decide what to do with any pre-release tag.

Another option would be to grab the version from chrome.runtime.getManifest() only for GUI.isChromeApp()

[mikeller]

For nwjs the call to chrome.runtime.getManifest() reads in package.json, I thought it was reading in manifest.json.

Yes, this is all wired up internally as it should be, so in that respect my hunch is to keep using it.

Could we modify the gulp system to also revise the version in manifest.json as part of the dist process? Unlike the blackbox viewer the dist process is required to run as a Chrome web app anyways as some files need to be moved around. I guess then we need to decide what to do with any pre-release tag.

Yes, that makes sense. The fact that the blackbox viewer happens to run out of the box if the modules in node_modules are all present is more of a side effect than desired - proper testing should involve re-running the build after changes.

Another option would be to grab the version from chrome.runtime.getManifest() only for GUI.isChromeApp()

Not sure why - getManifest() is the standardised way to get manifest information, and should be used. version.json was introduced because the manifest does not contain a git changeset id, and users were asking for this to be added. Maybe we can make this less confusing by renaming version.json to git_version.json or changeset.json?

[Docteh]

getManifest() isn't available to regular websites, so I'm trying to provide a work-around that tidys up the code just a hair.

How about we put the version in version.json, but only use it in cases where getManifest() is unavailable?

[mikeller]

Good point about website support - using version.json in this case sounds reasonable.

[Docteh]

Instead of
if(GUI.isChromeApp()) {
Switched to:
if(chrome.runtime && chrome.runtime.getManifest) {

So the package.json and manifest.json both have priority over version.json, also squashed it back down to 1 commit.

[mikeller]

We should probably respect version_name in here as well, like we did in getManifestVersion(), in case we want to do RC versions again at some point.

[Docteh]

@mikeller I like the idea of offering a pre-release build since there is an opt-in option for it. how about this?

I've added a comment to explain version_name, as it wasn't clear when I tore it out