flexget: 1.2.337 -> 2.8.17 #21636

Open
wants to merge 14 commits into
from

Projects

None yet

4 participants

@tari
Contributor
tari commented Jan 4, 2017
Motivation for this change

Flexget 1.2.337 isn't as old as it sounds (released July 2015) but this update does away with some dependencies on obsolete library versions.

Flexget's state does not upgrade from the old version successfully, so upgrading requires some manual intervention.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot

@tari, thanks for your PR! By analyzing the history of the files in this pull request, we identified @FRidh to be a potential reviewer.

@FRidh
Member
FRidh commented Jan 4, 2017

Thanks! Please put each package expression you touch in a separate commit instead one big commit like you have now.

@FRidh

As noted above, please split it in multiple commits.

Did you check whether tests are run for all packages with all interpreter versions?

This looks good and from the changes made I don't think this will break anything.

doCheck = false;
+ checkPhase = ''
+ py.test
+ '';
@FRidh
FRidh Jan 4, 2017 Member

maybe py.test flextest/tests/api_tests since that seems to be the only folder with tests.

@tari
tari Jan 4, 2017 Contributor

api_tests is the problematic directory:

flexget/tests/api_tests/test_server_api.py:13: in <module>
    from flexget.tests.conftest import MockManager
E   ImportError: No module named tests.conftest

Which in turn appears to make all of the tests in that directory fail with fixture 'api_client' not found.

@FRidh
FRidh Jan 4, 2017 Member

It seems a significant part of the test suite is not included
https://github.com/Flexget/Flexget/tree/develop/flexget/tests
You might want to fetch from GitHub instead of PyPI.

@tari
tari Jan 4, 2017 Contributor

Oh, interesting. I was referring to sources on github and didn't even consider there might be items missing from the pypi distribution. It works a little better when fetched from github, but requires additional (unpackaged) modules so I'll leave the tests disabled for now.

@@ -36,4 +43,4 @@ buildPythonPackage rec {
license = lib.licenses.mit;
maintainers = with lib.maintainers; [ domenkozar ];
@FRidh
FRidh Jan 4, 2017 Member

would you like to maintain this package?

@tari
Contributor
tari commented Jan 4, 2017

As noted above, please split it in multiple commits.

Done.

Did you check whether tests are run for all packages with all interpreter versions?

Is there a recommended non-manual way to do that?

@tari
Contributor
tari commented Jan 4, 2017

Did you check whether tests are run for all packages with all interpreter versions?

Is there a recommended non-manual way to do that?

I ran nox-review pr 21636 and made a few changes relating to py3k where the packages were broken.

Everything appears okay except zope_configuration and the packages that depend on it because of #20791. Most relevant to this PR among packages affected by #20791 is WSME which depends on cherrypy which was updated.

@Mic92
Contributor
Mic92 commented Jan 7, 2017

Maybe the necessary manual intervention could be added to the changelog.

@tari
Contributor
tari commented Jan 11, 2017

Added an item to release notes.

@FRidh anything else to be done? I think I've addressed all of your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment