IITC-CE / ingress-intel-total-conversion

intel.ingress.com total conversion user script with some new features. Should allow easier extension of the intel map.
https://iitc.app
ISC License
285 stars 110 forks source link

Scripts versioning #99

Closed johnd0e closed 4 years ago

johnd0e commented 5 years ago

Currently every version number contains date/time stamp as it's last component.

So with every build every script get newer version, even if it's content stays the same.

With this system if we turn on auto-updating feature - it will fire unconditionally, on every new build.

I don't think that it is good design, as typically we want to update changed scripts only.

angel93ayora commented 5 years ago

"ideal" solution would be to only change timestamp when first component is updated, complex because last component is calculated on build time and you need the last build to be able to read the last component (timestamp) you built time ago

another shoulution would be to not inject timestamp, or to do it only on test versions

johnd0e commented 5 years ago

another shoulution would be to not inject timestamp, or to do it only on test versions

Exactly.

modos189 commented 5 years ago

I'll add that we need to reduce the number of dots to the version number. Some userscripts managers report an error

// @version 0.3.0.20190103.162834

Required value' version' is missing or invalid. It must be between 1-4 dot-separated integers...

modos189 commented 5 years ago

another shoulution would be to not inject timestamp, or to do it only on test versions

I like this solution. And I need to remove point between date and time.

johnd0e commented 5 years ago

So I propose to remove .@@DATETIMEVERSION@@ completely, both for Release and Test-builds. May be we should add it to local builds (discussion needed).

johnd0e commented 5 years ago

So I propose to remove .@@DATETIMEVERSION@@ completely, both for Release and Test-builds.

But there is another option: what if 'datetime' would be not just 'build time', but actual timestamp of every file?

Bonus: it let us to (re)build only actually changed files. BTW, the idea is not new: https://github.com/IITC-CE/ingress-intel-total-conversion/blob/d77562b91a5b1e0bf170b4257eb33316a0b9daa3/build.py#L174-L176

gusowski1 commented 5 years ago

One reason where the @@datetimeversion@@ could make sense is the following use case.

Some code in the @@PLUGINEND@@ or @@PLUGINSTART@@ is changed the script itself is a newer version. In This case you need to bump the version of each script manually, otherwise the script itself won't be updated and the new code in the start and endsection wouldn't be changed.

One other way could be to change from @@datetimeversion@@ to something like the revison number. But that doesn't change the problem. As long as a build could change the scriptoutput we need some aditional parameter in the version of the script

johnd0e commented 5 years ago

There is not so simple with #135, but anyway I think that it would be better just to get rid of dateTimeVersion, completely.

@version already has 3 components, and it's enough for most purposes.

May be we can keep dateTimeVersion for development purposes, but then we should add it only for strictly specific targets, e.g. local or dev.

1valdis commented 5 years ago

@johnd0e

version already has 3 components, and it's enough for most purposes.

Agreed. That's how semantic versioning works, and I think you should stick with this too.

johnd0e commented 5 years ago

So I propose to remove timestamp component from release build. But it is still useful for local builds and test-builds.

Also I propose to remove garbage like this: // @description [local-2019-05-10-082357] ...