Issue 5668 - Replace 3rd component of version in AppxManifest.xml with build number

Issue 5668 - Replace 3rd component of version in AppxManifest.xml with build number

[Sebastian Noack, 2017-09-12 23:59:00]

https://codereview.adblockplus.org/29542845/diff/29542846/packagerEdge.py
File packagerEdge.py (left):

https://codereview.adblockplus.org/29542845/diff/29542846/packagerEdge.py#old...
packagerEdge.py:84: def create_appx_manifest(params, files,
release_build=False):
The default argument for "release_build" was redundant. There is no invocation
where it is not explicitly passed.

[tlucas, 2017-09-13 07:36:20]

LGTM

[tlucas, 2017-09-13 09:04:23]

This just popped into my mind - shouldn't the filename be adjusted too, for
consistency's sake?

[Oleksandr, 2017-09-13 11:26:37]

On 2017/09/13 09:04:23, tlucas wrote:
> This just popped into my mind - shouldn't the filename be adjusted too, for
> consistency's sake?

Yes, I think we should. It would mean the version in manifest.json would also
need to be changed. That way we'd have consistent version info in all the
places. For release builds we'd still have current versioning schema.

[Sebastian Noack, 2017-09-13 16:20:37]

On 2017/09/13 11:26:37, Oleksandr wrote:
> On 2017/09/13 09:04:23, tlucas wrote:
> > This just popped into my mind - shouldn't the filename be adjusted too, for
> > consistency's sake?
> 
> Yes, I think we should. It would mean the version in manifest.json would also
> need to be changed. That way we'd have consistent version info in all the
> places. For release builds we'd still have current versioning schema.

I was considering this. However, we have an inconsistency either way. Now the
package version (AppxManifest.json) uses a different scheme than the extension
version (manifest.json), but the latter is consistent across all target
platforms. Alternatively, if we make the extension version use the same scheme
as the package version, the extension version would be inconsistent with builds
for other browsers, and less accurate as well. Either way isn't ideal, but keep
that in mind. What do you think?

[Oleksandr, 2017-09-14 08:54:43]

On 2017/09/13 16:20:37, Sebastian Noack wrote:
> On 2017/09/13 11:26:37, Oleksandr wrote:
> > On 2017/09/13 09:04:23, tlucas wrote:
> > > This just popped into my mind - shouldn't the filename be adjusted too,
for
> > > consistency's sake?
> > 
> > Yes, I think we should. It would mean the version in manifest.json would
also
> > need to be changed. That way we'd have consistent version info in all the
> > places. For release builds we'd still have current versioning schema.
> 
> I was considering this. However, we have an inconsistency either way. Now the
> package version (AppxManifest.json) uses a different scheme than the extension
> version (manifest.json), but the latter is consistent across all target
> platforms. Alternatively, if we make the extension version use the same scheme
> as the package version, the extension version would be inconsistent with
builds
> for other browsers, and less accurate as well. Either way isn't ideal, but
keep
> that in mind. What do you think?

Whenever we ask users to report their ABP version we ask them to go look in Edge
settings, not in Windows Store. And Edge reports a version from manifest.json. I
guess for consistency with other browsers we can keep this as is indeed. Either
way is not ideal, and I don't have a strong preference. LGTM.

[tlucas, 2017-09-14 09:47:10]

On 2017/09/14 08:54:43, Oleksandr wrote:
> On 2017/09/13 16:20:37, Sebastian Noack wrote:
> > On 2017/09/13 11:26:37, Oleksandr wrote:
> > > On 2017/09/13 09:04:23, tlucas wrote:
> > > > This just popped into my mind - shouldn't the filename be adjusted too,
> for
> > > > consistency's sake?
> > > 
> > > Yes, I think we should. It would mean the version in manifest.json would
> also
> > > need to be changed. That way we'd have consistent version info in all the
> > > places. For release builds we'd still have current versioning schema.
> > 
> > I was considering this. However, we have an inconsistency either way. Now
the
> > package version (AppxManifest.json) uses a different scheme than the
extension
> > version (manifest.json), but the latter is consistent across all target
> > platforms. Alternatively, if we make the extension version use the same
scheme
> > as the package version, the extension version would be inconsistent with
> builds
> > for other browsers, and less accurate as well. Either way isn't ideal, but
> keep
> > that in mind. What do you think?
> 
> Whenever we ask users to report their ABP version we ask them to go look in
Edge
> settings, not in Windows Store. And Edge reports a version from manifest.json.
I
> guess for consistency with other browsers we can keep this as is indeed.
Either
> way is not ideal, and I don't have a strong preference. LGTM.

Imho consistency across different browsers is more valuable than coping
Microsoft
versioning conventions all across our code base.

Concluding: still LGTM