@worldofpeace I have removed pythonPackages.pydot_ng from the PR. Ready for merge. I have nix-review running and everything looks good so far (about 50% done)

@FRidh I feel like the situation with the hashes being base32 probably isn't something to document specifically for python packages, but perhaps in general.

As for about pytest, I'm not sure if we've documented the situation with not mixing versions globally in
9.14.4.1. Contributing guidelines but it would be easy to add a note about pytest packages if something like that was there.

@costrouc When you supply the fixes for the requested changes it would help reviewers if you did easily fixup'able commits so the changes can easily be seen on github. That way I don't have to start over with the review, even if we're certain we got everything.

Otherwise LGTM, be sure to paste your nix-review report in a gist.

@worldofpeace you mean contrary to fixing up the commits directly? I always request for that instead, so I can just merge when I need to, instead of asking yet another time for squashing them. I guess there's pros and cons to both.

@FRidh That's because it's convenient for me to just fixup the authors history real quick and merge once I know it's good.

@worldofpeace so I should create a fixup commit after all the packages? Right now I do some rebase/fixup trickery so that I can keep each commit separate and dealing with a single package each.

@costrouc Essentially

Original commit

pythonPackages.jsonschema: 2.6.0 -> 3.0.1

"Please drop x input not needed" on jsonschema

pythonPackages.jsonschema: drop x input

Or even just a fixup! commit.
Is it uncommon for people to do this?

If I follow you are suggesting that I create additional commits and you will handle combining commits once the PR is ready for merge. I will do that from now on if there are any more recommended changes.

Yeah essentially, I think @FRidh is suggesting that it just adds an extra step for merge since you're waiting for the author to fix the history.

As @worldofpeace, I prefer fixup commits but I don't think it's possible from github's ui to squash the fixup ? does the rebase do that ?

@teto Oh no, I checkout the pr locally and just force push that.

Ready for merge. Here are the logs of nix-review https://gist.github.com/costrouc/b5bd51c2736378c0e5a3c5831e085fbd

I didn't wait for the nix-review to finish because I've been waiting on the last 5 packages for about 2 hours

@worldofpeace of peace is it normal for so many packages to not build in a nix-review?

@costrouc Some of the failures could have been caused by bf54079.

But we should be able to use https://hydra.nixos.org/jobset/nixpkgs/python-unstable to figure this out.
I think we're good for merge.

This broke aws-sam-translator which many packages depend on https://hydra.nixos.org/eval/1530873. Due to jsonschema as you expected

I believe this release fixes that

• https://github.com/awslabs/serverless-application-model/releases/tag/v1.12.0

I'll submit a PR for that