Skip to content

update colorama and related #26311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 3, 2017
Merged

update colorama and related #26311

merged 4 commits into from
Jun 3, 2017
+81 −68

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Jun 1, 2017

Motivation for this change
Things done
  • Tested using sandboxing
  • 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" (treq is failing but seems unrelated)
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

FRidh
FRidh requested changes Jun 2, 2017
View reviewed changes
Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move all expressions to separate files.

pkgs/top-level/python-packages.nix Outdated

colorama = buildPythonPackage rec {
name = "colorama-${version}";
colorama37 = self.colorama.overrideAttrs (old: rec {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • only a single version of a package in pythonPackages. If you really need another version, include it via a let ... in expression. Only if multiple packages need this extra version, we introduce it globally.
  • when appending a version number, its supposed to be colorama_3_7 to avoid ambiguation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I could probably remove the check since the release it's just bugfixes but the developer of awscli says colorama had been breaking the program before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in colorama have indeed caused breakage in the past. Let's leave such an optimization to the awscli maintainer.

rnhmjoj added 3 commits June 2, 2017 11:57
@rnhmjoj
pythonPackages.colorama: move to development/python-modules

Verified

This commit was signed with the committer’s verified signature.
eadwu Edmund Wu
GPG key ID: 8AB0E99F622EC03E
Verified
Learn about vigilant mode
28cbb4a
@rnhmjoj
pythonPackages.botocore: 1.5.38 -> 1.5.58

Verified

This commit was signed with the committer’s verified signature.
eadwu Edmund Wu
GPG key ID: 8AB0E99F622EC03E
Verified
Learn about vigilant mode
ffedf3f
@rnhmjoj
tmuxp: 1.2.7 -> 1.3.1
805ee69
FRidh
FRidh approved these changes Jun 2, 2017
View reviewed changes
Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haven't tested it but looks good.

@Mic92
Copy link
Member

Mic92 commented Jun 3, 2017

When I starting tmuxp, I get the following error message:

Traceback (most recent call last):
  File "/nix/store/zphh0zdij5mjv1hccmd2lp7x746lkl5n-tmuxp-1.3.1/bin/.tmuxp-wrapped", line 8, in <module>
    from tmuxp import cli
  File "/nix/store/zphh0zdij5mjv1hccmd2lp7x746lkl5n-tmuxp-1.3.1/lib/python2.7/site-packages/tmuxp/__init__.py", line 20, in <module>
    from . import config, util, cli
  File "/nix/store/zphh0zdij5mjv1hccmd2lp7x746lkl5n-tmuxp-1.3.1/lib/python2.7/site-packages/tmuxp/cli.py", line 17, in <module>
    from libtmux.common import has_minimum_version, which
ImportError: cannot import name has_minimum_version

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jun 3, 2017

libtmux was too old. For one time the requirements.txt constraint was necessary.

@Mic92 Mic92 merged commit 4c03492 into NixOS:master Jun 3, 2017
@rnhmjoj rnhmjoj deleted the colorama branch September 12, 2017 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rnhmjoj @Mic92 @FRidh