1023913 - Upgrade dependency from ManifestDestiny to manifestparser

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

On bug 984528 the manifest destiny package has been renamed to manifestparser. We should update the setup.py file in Mozmill to make use of the new package.

Comment 1

[Anna]

Attachment: 0001-Bug-1023913-Upgrade-dependency-to-manifestparser.patch

Comment 2

[Anna]

Ahh wrong patch :/

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Anna from comment #2)
> Ahh wrong patch :/

Hi Anna, anything wrong here? Can you please clarify your last comment? Btw, thanks for your first patch! I assume I can mark you as assignee of this bug? :)

Comment 4

[Anna]

Attachment: Bug-1023913-Rename-ManifestDestiny-to-manifestparser.patch

Will this work ?

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8444596 [details] [diff] [review]
Bug-1023913-Rename-ManifestDestiny-to-manifestparser.patch

Review of attachment 8444596 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like the correct way, but please address the two comments as seen inline. Also please update the commit message to mention myself (hskupin) as reviewer. Thanks.

::: mozmill/setup.py
@@ +9,4 @@
>  PACKAGE_VERSION = "2.1-dev"
>  
>  deps = ['jsbridge == 3.1-dev',
> +        'manifestparser >= 0.6',

Please keep the equal signs. Otherwise we might run into trouble when manifestparser gets an API change, and we install that new version.

::: mutt/setup.py
@@ +13,1 @@
>          'mozfile',

Same here. Maybe could you even add the specific version to the latest mozfile package?

Comment 6

[Anna]

Attachment: Bug-1023913-Rename-ManifestDestiny-to-manifestparser.patch

Not sure if I got the version right for the mozfile package

Comment 7

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

:whimboo, can you review this?

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8445274 [details] [diff] [review]
Bug-1023913-Rename-ManifestDestiny-to-manifestparser.patch

Review of attachment 8445274 [details] [diff] [review]:
-----------------------------------------------------------------

Sure, not sure why I missed that review request. Thanks for pinging, Joel.

Anna, please see my comments inline. Beside those have you tested the changes by running a complete Mutt testrun? For details how to do this please see https://github.com/mozilla/mozmill/blob/master/mutt/README.md

::: mutt/setup.py
@@ +9,5 @@
>  PACKAGE_NAME = "mutt"
>  PACKAGE_VERSION = "0.1"
>  
> +deps = ['manifestparser == 0.6',
> +        'mozfile == 0.1',

Mozfile is meanwhile at version 1.1. Also we should not go back before 1.0, given that it would cause API conflicts.

Comment 9

[Anna]

Attachment: 0001-PATCH-Bug-1023913-Rename-ManifestDestiny-to-manifest.patch

Sorry it took so long btw I ran mutt tests and I'm getting "AttributeError: 'ProcessHandler' object has no attribute 'proc'"

Comment 10

[Anna]

Attachment: 0001-Bug-1023913-Rename-ManifestDestiny-to-manifestparser.patch

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8507627 [details] [diff] [review]
0001-Bug-1023913-Rename-ManifestDestiny-to-manifestparser.patch

Review of attachment 8507627 [details] [diff] [review]:
-----------------------------------------------------------------

We have a bit of breakage with dependencies on our master branch, so mozmill doesn't run at all at the moment. But this change does no harm, so lets get this in.

Thanks Anna for your contribution. If you are interested to learn and do more please let me know and we can find something else for you.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Landed on master for 2.1 release:
https://github.com/mozilla/mozmill/commit/7bbd9923a33ae5470a5009bf9cd9b367356bcdf2