Issue 5666 - Replace 4th component of version in AppxManifest.xml with 0

Issue 5666 - Replace 4th component of version in AppxManifest.xml with 0

[Sebastian Noack, 2017-09-12 19:52:33]

https://codereview.adblockplus.org/29542744/diff/29542745/tests/test_packager...
File tests/test_packagerEdge.py (right):

https://codereview.adblockplus.org/29542744/diff/29542745/tests/test_packager...
tests/test_packagerEdge.py:216: {'metadata': metadata, 'version': '1.2.3.4'},
The previous change broke this test already, since params['version'] is no
longer overridden with a value from metadata.

[tlucas, 2017-09-12 21:24:18]

Hey Sebastian,

this looks alright - just one stylistic nit below.

Best,
Tristan

https://codereview.adblockplus.org/29542744/diff/29542745/packagerEdge.py
File packagerEdge.py (right):

https://codereview.adblockplus.org/29542744/diff/29542745/packagerEdge.py#new...
packagerEdge.py:76: four components with the last component being zero (e.g.
12.34.56.0).
You should use the imperative for docstrings, e.g. "Prepare a version number..."


In case you really need a multi-line docstring, you should also add a one-line
summary and a blank line, just before the multi-line description.

Or you could transform it into a one-liner, e.g.:
    """Prepare a version number to adhere to the required pattern x.x.x.0."""

What do you think about this?

https://codereview.adblockplus.org/29542744/diff/29542745/tests/test_packager...
File tests/test_packagerEdge.py (right):

https://codereview.adblockplus.org/29542744/diff/29542745/tests/test_packager...
tests/test_packagerEdge.py:216: {'metadata': metadata, 'version': '1.2.3.4'},
On 2017/09/12 19:52:33, Sebastian Noack wrote:
> The previous change broke this test already, since params['version'] is no
> longer overridden with a value from metadata.

Ack, tox succeeds happily now. (I also added "tox" to my review-cheat-sheet)

[Sebastian Noack, 2017-09-12 21:35:46]

https://codereview.adblockplus.org/29542744/diff/29542745/packagerEdge.py
File packagerEdge.py (right):

https://codereview.adblockplus.org/29542744/diff/29542745/packagerEdge.py#new...
packagerEdge.py:76: four components with the last component being zero (e.g.
12.34.56.0).
On 2017/09/12 21:24:18, tlucas wrote:
> You should use the imperative for docstrings, e.g. "Prepare a version
number..."
> 
> 
> In case you really need a multi-line docstring, you should also add a one-line
> summary and a blank line, just before the multi-line description.
> 
> Or you could transform it into a one-liner, e.g.:
>     """Prepare a version number to adhere to the required pattern x.x.x.0."""
> 
> What do you think about this?

Done.

[tlucas, 2017-09-12 21:38:08]

https://codereview.adblockplus.org/29542744/diff/29542810/packagerEdge.py
File packagerEdge.py (right):

https://codereview.adblockplus.org/29542744/diff/29542810/packagerEdge.py#new...
packagerEdge.py:75: 
Flake8 fails on this line:
./packagerEdge.py:75:1: W293 blank line contains whitespace

[Sebastian Noack, 2017-09-12 21:41:18]

https://codereview.adblockplus.org/29542744/diff/29542810/packagerEdge.py
File packagerEdge.py (right):

https://codereview.adblockplus.org/29542744/diff/29542810/packagerEdge.py#new...
packagerEdge.py:75: 
On 2017/09/12 21:38:08, tlucas wrote:
> Flake8 fails on this line:
> ./packagerEdge.py:75:1: W293 blank line contains whitespace

Argh, sorry, I forgot to run tox/flake8, again. Apparently just changing a
docstring could still cause warnings. Fixed.

[tlucas, 2017-09-12 21:42:39]

No problem - fwiw it's some practice for me :)

LGTM