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

add new eval checker for the package list with allowAliases = false #250

Merged
merged 1 commit into from Oct 15, 2018

Conversation

ryantm
Copy link
Member

@ryantm ryantm commented Oct 14, 2018

closes #248

By checking the package list without aliases, we can make it more
likely that people do not introduce new aliases into nixpkgs without
removing them from the repository. Aliases are supposed to be for
backward compatibility.

Also, we can make sure this configuration option continues to work for
people. Noteably, this configuration option is used by
r-ryantm/nixpkgs-update to avoid submitting duplicate pull requests
for the package and its alias, and to use the correct (non-alaised)
name in the pull request.

@@ -331,6 +331,19 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
self.nix.clone()
),

EvalChecker::new(
"package-list-no-aliases)",
Copy link
Member

Choose a reason for hiding this comment

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

Looks like parenthesis mismatch.

closes NixOS#248

By checking the package list without aliases, we can make it more
likely that people do not introduce new aliases into `nixpkgs` without
removing them from the repository. Aliases are supposed to be for
backward compatibility.

Also, we can make sure this configuration option continues to work for
people. Noteably, this configuration option is used by
r-ryantm/nixpkgs-update to avoid submitting duplicate pull requests
for the package and its alias, and to use the correct (non-alaised)
name in the pull request.
@ryantm
Copy link
Member Author

ryantm commented Oct 14, 2018

@7c6f434c fixed! Thanks for reviewing.

@LnL7
Copy link
Member

LnL7 commented Oct 15, 2018

@grahamc Do you think this would be ok in terms of resource usage, given that it adds an extra evaluation to each merge event?

@grahamc
Copy link
Member

grahamc commented Oct 15, 2018

I think yes, let's see how it goes. We can change the architecture of this process if we need to. This is something we should check, since it prevents problems from reaching Hydra.

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

Successfully merging this pull request may close these issues.

feature request: check evaluation succeeds with allowAliases = false
4 participants