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

teller: 1.5.6 -> 2.0.7 #314308

Merged
merged 1 commit into from
May 29, 2024
Merged

teller: 1.5.6 -> 2.0.7 #314308

merged 1 commit into from
May 29, 2024

Conversation

cameronraysmith
Copy link
Member

@cameronraysmith cameronraysmith commented May 24, 2024 •
edited
Loading

Description of changes

From 1.5.6 to 2.0.7 teller, previously written in go, was rewritten in rust.
tellerops/teller@v1.5.6...v2.0.7
tellerops/teller#216
This PR updates the derivation to account for this change.

As noted in the teller docs, the teller test suite depends on node and docker. It was disabled in favor of a "smoke test" installCheckPhase, but it would obviously be better to attempt to reenable the upstream tests in the future.

I also added myself as a maintainer in addition to @wahtique .

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.

@cameronraysmith cameronraysmith marked this pull request as ready for review May 24, 2024 15:07
@cameronraysmith cameronraysmith requested a review from wahtique May 24, 2024 15:07
@ofborg ofborg bot requested a review from wahtique May 24, 2024 15:45
@cameronraysmith cameronraysmith force-pushed the update-teller-207 branch 2 times, most recently from f60bd94 to 9f406f0 Compare May 25, 2024 00:40
@cameronraysmith
teller: 1.5.6 -> 2.0.7
fca39cf 
Signed-off-by: Cameron Smith <[email protected]>
@cameronraysmith
Copy link
Member Author

Result of nixpkgs-review pr 314308 run on aarch64-darwin 1

1 package built:
  • teller

@cameronraysmith cameronraysmith self-assigned this May 27, 2024
nbraud
nbraud approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@nbraud nbraud left a comment

Choose a reason for hiding this comment

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

Great job overall ❤️
LMK if you want to apply my suggestion, want me to do it prior to merging, or don't want it at all.

pkgs/development/tools/teller/default.nix Show resolved Hide resolved
@nbraud
Copy link
Contributor

nbraud commented May 28, 2024

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

1 package built:
  • teller

@cameronraysmith
Copy link
Member Author

cameronraysmith commented May 28, 2024 •
edited
Loading

@nbraud I agree that your proposal to use testers.versionCheck would be better than the manual installCheckPhase, but I would propose to consider migrating to this approach in a future PR where we attempt to reenable the selection of tests in the checkPhase that can be executed successfully in the nix sandbox. Definitely feel free to counter if you feel strongly this should be better resolved prior to merging this PR or know of a simpler way of resolving it than I can currently imagine ;).

@cameronraysmith cameronraysmith requested a review from nbraud May 28, 2024 15:13
@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels May 29, 2024
@nbraud
Copy link
Contributor

nbraud commented May 29, 2024 •
edited
Loading

I agree [...] but I would propose to consider migrating to this approach in a future PR

That's fine by me as well, if you prefer to do that in a follow-up PR.

PS: I'm somewhat confused by the request for a new review, didn't I already approve the PR in its current state?

@nbraud nbraud merged commit 23381e7 into NixOS:master May 29, 2024
34 checks passed
@nbraud nbraud removed the 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package label May 29, 2024
cameronraysmith added a commit to cameronraysmith/nix-config that referenced this pull request Jun 3, 2024
@cameronraysmith
fix(overlay): revert to upstream teller
22c9415 
NixOS/nixpkgs#314308
Signed-off-by: Cameron Smith <[email protected]>
florian-sanders-cc pushed a commit to florian-sanders-cc/nixpkgs that referenced this pull request Jun 6, 2024
@cameronraysmith @florian-sanders-cc
teller: 1.5.6 → 2.0.7 (NixOS#314308)
c71447f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wahtique wahtique Awaiting requested review from wahtique

@nbraud nbraud Awaiting requested review from nbraud

Assignees

@cameronraysmith cameronraysmith

Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update request: teller 1.5.6 → 2.0.7
4 participants
@cameronraysmith @nbraud @wahtique @wegank