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

make allocpool check the allocations file #3839

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JanZerebecki
Copy link
Contributor

as sometimes people change allocations, but not the contents of the pool
directory or the other way around. Make it check both, to ensure
someones running mkcloud is not replaced.

jgrassler
jgrassler previously approved these changes Mar 18, 2020
Copy link
Contributor

@jgrassler jgrassler left a comment

Choose a reason for hiding this comment

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

Looks sound from a functional point of view. Feel free to address my nit or leave it as it is :-)

my $n=$d;
$n=~ s/$pooldir\///;
while(<$fha>) {
if(/^[a-z]*$n jenkins$/) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you could use an explicit loop variable here for readability. I suspect that most people who don't deal with Perl on a regular basis might not be familiar with the subtleties of $_ :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, please re-review

as sometimes people change allocations, but not the contents of the pool
directory or the other way around. Make it check both, to ensure
someones running mkcloud is not replaced.
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.

2 participants