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

Re #11081 Moved log.print command outside of loop #6

Merged
merged 3 commits into from
Feb 16, 2015

Conversation

lottiegreenwood
Copy link

No description provided.

@FedeMPouzols FedeMPouzols self-assigned this Feb 16, 2015
@FedeMPouzols
Copy link
Contributor

This looks ok, just one comment: before, the 'wait for a second' had a very literal interpretation. Now that the print is outside the loop maybe something like 'waiting until some processes finish' or 'waiting until processors become available' would be more clear? Let me know and I'll merge this in.

Also, related to this I'd suggest to put the '4' as a variable, more visible somewhere. I'd guess at some point this will be a configuration option?

@lottiegreenwood
Copy link
Author

Good point on the log message, I will change it to something less decisive. Re: the 4 processes allowed, I believe it will be set as a particular number that no scripts will be able to configure, just the admin. I will put it as a more identifiable variable somewhere though, thanks.

FedeMPouzols added a commit that referenced this pull request Feb 16, 2015
…log_message

Re #11081 Moved log.print command outside of loop
@FedeMPouzols FedeMPouzols merged commit f4fe414 into master Feb 16, 2015
@FedeMPouzols FedeMPouzols deleted the 11081_Improve_autoreduction_log_message branch February 16, 2015 12:53
@FedeMPouzols
Copy link
Contributor

This fixed ticket #11081.

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.

None yet

2 participants