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.

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

testers: add more parameters, add testVersion to my maintained packages #306307

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Atemu
Copy link
Member

@Atemu Atemu commented Apr 23, 2024

Description of changes

When any aspect of the command to show the version needed to change, the entire
command would have to be declared in full. This allows directly customising
those aspects individually.

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@roberth roberth added the 6.topic: testing Tooling for automated testing of packages and modules label Apr 24, 2024
nbraud
nbraud previously requested changes May 2, 2024
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 work! I do have a little feedback, but other than that and needing tests I'd approve it.

pkgs/build-support/testers/default.nix Show resolved Hide resolved
pkgs/build-support/testers/default.nix Outdated Show resolved Hide resolved
doc/build-helpers/testers.chapter.md Outdated Show resolved Hide resolved
@Atemu
Copy link
Member Author

Atemu commented May 8, 2024

Thank you all for the feedback, I still intend to continue this but it may take a bit.

@nbraud
Copy link
Contributor

nbraud commented May 12, 2024

Thank you all for the feedback, I still intend to continue this but it may take a bit.

If you are too busy, or are getting overwhelmed in any way, I can also address the feedback myself and merge.

It's also fine if you want to do it yourself and need time for whatever reason, I just wanted to make clear you have the option to let us do the rest: after all, teamwork makes the dream work ❤️

@Atemu
Copy link
Member Author

Atemu commented May 13, 2024

I'm not overwhelmed or anything, it's just not very high on my priority list right now. If anyone wants to take over this (or any of my other PRs for that matter), they should feel absolutely welcome to do so. You're not stealing anything away from me here ;)

Atemu added 8 commits May 26, 2024 13:26
When any aspect of the command to show the version needed to change, the entire
command would have to be declared in full. This allows directly customising
those aspects individually.
It's not that important but this is minimally more useful than an empty file.
buildGoModule does not support finalAttrs
Some programs exit with non-zero exit code for some reason. It should still be
possible to run the version smoke check in that case.
@nbraud
Copy link
Contributor

nbraud commented May 26, 2024

Rebased to address merge conflict

@github-actions github-actions bot removed the 6.topic: testing Tooling for automated testing of packages and modules label May 26, 2024
@nbraud nbraud added 6.topic: testing Tooling for automated testing of packages and modules and removed 2.status: merge conflict labels May 26, 2024
@github-actions github-actions bot removed the 6.topic: testing Tooling for automated testing of packages and modules label May 26, 2024
@nbraud nbraud dismissed their stale review May 26, 2024 13:38

Addressed my own comments

@nbraud
Copy link
Contributor

nbraud commented May 26, 2024

For ease of review, I avoided introducing changes in @Atemu's commits, but the fixup! commit will need to be squashed in before merging.

@roberth, could you review my changes? Are the tests sufficient?

@Atemu
Copy link
Member Author

Atemu commented May 26, 2024

If the commits touching my packages are causing any issue, feel free to drop them. I can put those in a separate PR following this one.

@nbraud
Copy link
Contributor

nbraud commented May 26, 2024

If the commits touching my packages are causing any issue, feel free to drop them. I can put those in a separate PR following this one.

They aren't, don't worry <3
The only fixup! commit is applying one of the suggestions I made during the first round of review

@nbraud
Copy link
Contributor

nbraud commented May 26, 2024

The build issue on x86_64-darwin seems unrelated:

error: a 'x86_64-linux' with features {} is required to build '/nix/store/0w4d3xh6ykidx2k473djpxplnhyp8ph9-Xresources-Xft.drv', but I am a 'x86_64-darwin' with features {benchmark, big-parallel, nixos-test}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants