Skip to content
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

aws-sam-cli: 1.6.2 -> 1.14.0 #104782

Merged
merged 2 commits into from Feb 12, 2021
Merged

aws-sam-cli: 1.6.2 -> 1.14.0 #104782

merged 2 commits into from Feb 12, 2021

Conversation

freezeboy
Copy link
Contributor

Motivation for this change

Upstream update

Should wait after #103742

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from lo1tuma November 24, 2020 18:27
@SuperSandro2000
Copy link
Member

Can you please target staging?

@freezeboy freezeboy changed the base branch from master to staging November 28, 2020 09:01
@freezeboy freezeboy marked this pull request as ready for review November 28, 2020 09:02
@ofborg ofborg bot added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Nov 28, 2020
@ofborg ofborg bot requested a review from andreabedini January 4, 2021 15:04
@davegallant davegallant mentioned this pull request Jan 10, 2021
Open
@jonringer
Copy link
Contributor

oh, sorry, I fixed the current build, and that caused some conflicts

@freezeboy freezeboy requested a review from peti as a code owner January 18, 2021 07:55
@freezeboy
Copy link
Contributor Author

Rebased + conflict resolved 🤞

@freezeboy freezeboy changed the base branch from staging to master January 18, 2021 07:56
@hirenashah
Copy link
Contributor

@freezeboy You might want to bump it once more: https://github.com/aws/aws-sam-cli/releases/tag/v1.14.0

SuperSandro2000
SuperSandro2000 reviewed Jan 23, 2021
View reviewed changes
pkgs/development/tools/aws-sam-cli/default.nix Outdated
in

with py.pkgs;
with python3.pkgs;
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer moving this to where it is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is okay, mostly because it's not mixing toolchains. Since it's a "standard" python package, I think the context is apparent enough. However, if it was also doing some c++/rust builds, and something more complicated; then I would agree that scoping is vital.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I admit I simply replaced the with call as no more override was needed

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 104782 run on x86_64-linux 1

1 package built:
  • aws-sam-cli

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

aws-sam-cli:

Please consider this feature to be alpha.

A substituteInPlace with an unmatched pattern got detected:

substituteStream(): WARNING: pattern 'serverlessrepo==0.1.9' doesn't match anything in file 'requirements/base.txt'

Please check the offending substituteInPlace for typos or changes in source.

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 104782 run on x86_64-darwin 1

1 package built:
  • aws-sam-cli

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

aws-sam-cli:

Please consider this feature to be alpha.

A substituteInPlace with an unmatched pattern got detected:

substituteStream(): WARNING: pattern 'serverlessrepo==0.1.9' doesn't match anything in file 'requirements/base.txt'

Please check the offending substituteInPlace for typos or changes in source.

@freezeboy freezeboy changed the title aws-sam-cli: 1.6.2 -> 1.13.1 aws-sam-cli: 1.6.2 -> 1.14.0 Jan 30, 2021
@freezeboy
Copy link
Contributor Author

@hirenashah Ok updated ... again

Really sad this trivial change is pending since November 2020 ...

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 104782 run on x86_64-darwin 1

1 package built:
  • aws-sam-cli

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

aws-sam-cli:

Please consider this feature to be alpha.

A substituteInPlace with an unmatched pattern got detected:

substituteStream(): WARNING: pattern 'serverlessrepo==0.1.9' doesn't match anything in file 'requirements/base.txt'

Please check the offending substituteInPlace for typos or changes in source.

@SuperSandro2000
Copy link
Member

If the stale substituteInPlace is removed this can be merged. The version bump could also have been a new PR.

@a-h
Copy link
Contributor

a-h commented Feb 11, 2021

So the remaining task is simply to delete this one line?

      --replace "serverlessrepo==0.1.9" "serverlessrepo~=0.1.9" \

@adisbladis adisbladis removed their request for review February 11, 2021 13:06
@SuperSandro2000
Copy link
Member

So the remaining task is simply to delete this one line?

      --replace "serverlessrepo==0.1.9" "serverlessrepo~=0.1.9" \

I think so.

@andreabedini andreabedini removed their request for review February 12, 2021 06:38
@freezeboy
Copy link
Contributor Author

Rebased, dangling substition removed, built locally, and force pushed

@SuperSandro2000 SuperSandro2000 merged commit 668bc71 into NixOS:master Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@lo1tuma lo1tuma Awaiting requested review from lo1tuma

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

@ttuegel ttuegel Awaiting requested review from ttuegel

@peti peti Awaiting requested review from peti

Assignees
No one assigned
Labels
2.status: blocked by pr/issue 6.topic: python 8.has: clean-up 8.has: package (new) 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@freezeboy @SuperSandro2000 @a-h @jonringer @hirenashah