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

Switch the pkgs/by-name check to a separate repository #297901

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Mar 22, 2024

Description of changes

This completes the move of the pkgs/by-name tooling to the new https://github.com/NixOS/nixpkgs-check-by-name repository, see #286559 for motivation.

Note

I'm leaving in both pkgs/test/nixpkgs-check-by-name to make it easier to review the removal in a separate PR, and pkgs/test/nixpkgs-check-by-name/scripts/pinned-tool.json because it's needed for CI of this PR to succeed.

This work is sponsored by Antithesis

Things done

  • Test it in CI: Test tweag/nixpkgs#87
  • Test it locally: ./maintainers/scripts/check-by-name.sh master
  • Update all comments (especially in the workflow)

Add a 👍 reaction to pull requests you find important.

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Looks good.

@@ -108,13 +108,13 @@ There's some limitations as to which packages can be defined using this structur

## Validation

CI performs [certain checks](../test/nixpkgs-check-by-name/README.md#validity-checks) on the `pkgs/by-name` structure.
This is done using the [`nixpkgs-check-by-name` tool](../test/nixpkgs-check-by-name).
CI performs [certain checks](https://github.com/NixOS/nixpkgs-check-by-name?tab=readme-ov-file#validity-checks) on the `pkgs/by-name` structure.
Copy link
Contributor

Choose a reason for hiding this comment

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

Future: we should publish with GitHub pages so that https://nixpkgs-check-by-name.github.io/ is the URL.

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't seem necessary, but yeah that's an option :)

@infinisil infinisil marked this pull request as draft March 26, 2024 20:13
@infinisil
Copy link
Member Author

Drafted because I noticed a minor problem that would occur with the follow-up removal PR, where I wanted to be able to remove pkgs/test/nixpkgs-check-by-name but persist its ./scripts in pkgs/test/check-by-name. Would cause a CI failure though unless I'd delay moving pkgs/test/nixpkgs-check-by-name/scripts/pinned-version.txt for another PR. I'll do the alternative of moving it now, so that the follow-up PR will only remove things.

The nixpkgs-check-by-name tooling is [being moved](NixOS#286559 (comment))
to a [separate repo](https://github.com/NixOS/nixpkgs-check-by-name).

This commit updates Nixpkgs CI to use it instead of the tree inside
Nixpkgs

No changes have been made to the tooling locally since it was moved:
- [Exported history](https://github.com/NixOS/nixpkgs/commits/55bf02190ee57fcf83490fd7b6bf7834e28c9c86/pkgs/test/nixpkgs-check-by-name)
- [Imported history](https://github.com/NixOS/nixpkgs-check-by-name/commits/d579e1821d56c79fd90dab34b991cc7bdab7a5c6/)
@infinisil infinisil marked this pull request as ready for review March 26, 2024 20:25
@infinisil
Copy link
Member Author

Force pushed with the necessary changes (diff)

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Yeah, let's go x2

@infinisil infinisil merged commit 8a1110b into NixOS:master Mar 26, 2024
6 of 7 checks passed
infinisil added a commit that referenced this pull request Mar 26, 2024
It was not removed in #297901 so
that CI for that PR itself would not fail since CI runs from the base
branch.
infinisil added a commit that referenced this pull request Mar 26, 2024
Since #297901, the tool is fetched
from https://github.com/NixOS/nixpkgs-check-by-name, so there's no need
to keep it around in Nixpkgs anymore
@infinisil infinisil deleted the check-by-name-separate-repo branch April 5, 2024 14:12
infinisil added a commit to tweag/nixpkgs that referenced this pull request Apr 10, 2024
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.

None yet

2 participants