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

python312Package.{fabric,paramiko}: modernize and fix compilation #351372

Closed
wants to merge 2 commits into from
Closed

python312Package.{fabric,paramiko}: modernize and fix compilation #351372

wants to merge 2 commits into from

Conversation

pluiedev
Copy link
Contributor

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

fabaff
fabaff reviewed Oct 26, 2024
View reviewed changes
Copy link
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

This PR should be rebased. Some of the change for paramiko are already present in staging-next.

@ofborg ofborg bot requested review from Conni2461 and dasJ October 26, 2024 10:48
@pluiedev
Copy link
Contributor Author

pluiedev commented Oct 26, 2024 •
edited
Loading

This PR should be rebased. Some of the change for paramiko are already present in staging-next.

Rebased, however, I didn't see any merge conflicts — are you sure the changes I made were already in staging?

EDIT: I am stupid

@pluiedev pluiedev force-pushed the fix/fabric-paramiko branch from c53c9bf to 9879aca Compare October 26, 2024 17:04
@pluiedev pluiedev marked this pull request as draft October 26, 2024 17:14
@pluiedev pluiedev force-pushed the fix/fabric-paramiko branch from 9879aca to 4fc45cf Compare October 26, 2024 17:45
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell 6.topic: qt/kde 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: printing 6.topic: rust 6.topic: policy discussion 6.topic: golang 6.topic: ruby 6.topic: vim 6.topic: stdenv Standard environment 6.topic: nodejs 6.topic: cinnamon Desktop environment 6.topic: systemd labels Oct 26, 2024
@github-actions github-actions bot removed 6.topic: ruby 6.topic: vim 6.topic: stdenv Standard environment 6.topic: nodejs 6.topic: cinnamon Desktop environment 6.topic: systemd 6.topic: vscode 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 6.topic: php 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related labels Oct 26, 2024
@pluiedev pluiedev force-pushed the fix/fabric-paramiko branch from 4fc45cf to 4f779b7 Compare October 26, 2024 17:47
@pluiedev pluiedev marked this pull request as ready for review October 26, 2024 17:51
Scrumplex
Scrumplex approved these changes Nov 5, 2024
View reviewed changes
Copy link
Member

@Scrumplex Scrumplex left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 5, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4819

Scrumplex
Scrumplex reviewed Nov 10, 2024
View reviewed changes
Copy link
Member

@Scrumplex Scrumplex left a comment

Choose a reason for hiding this comment

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

I think this is technically good to merge. I think it would be best to split your commit into two though, as you are modifying two derivations here.

@pluiedev pluiedev force-pushed the fix/fabric-paramiko branch from 4f779b7 to e0bc00d Compare November 10, 2024 14:11
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 10, 2024
@SuperSandro2000
Copy link
Member

@ofborg build python312Packages.fabric python312Packages.linien-client Fabric azure-cli

@getchoo getchoo added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 8, 2024
@pluiedev
Copy link
Contributor Author

Seems like 06075b4 already implements most things I've done here — closing

@pluiedev pluiedev closed this Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Scrumplex Scrumplex Scrumplex approved these changes

@fabaff fabaff fabaff left review comments

@Conni2461 Conni2461 Awaiting requested review from Conni2461

@dasJ dasJ Awaiting requested review from dasJ

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: python 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@pluiedev @nixos-discourse @SuperSandro2000 @fabaff @Scrumplex @wegank @getchoo