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

JENKINS-2673, JENKINS-1406 Fix inappropriate cron warnings #676

Closed
wants to merge 1 commit into from

Conversation

RichardBradley
Copy link

As the other commenters on https://issues.jenkins-ci.org/browse/JENKINS-2673 found, I was surprised to see a slightly confusing warning from Jenkins when I set my update schedule to "* * * * *".

This patch should:

  • Not issue the minutes / hours warning in the "* * * * *" case, as it seems inappropriate
  • Improve the warning in the cases where it is appropriate (see JENKINS-1406 for motivating examples)

I wasn't able to run the unit tests: would it be possible for whoever picks this up to run the tests before merging?

Thanks,

Rich

@kutzi
Copy link
Member

kutzi commented Mar 3, 2013

I think this needs a unit test. I find this method quite hard to understand - mainly already because of the existing code and not your changes. But anyway a unit test would greatly help to document which pattern(s?) should be warned about and which not.

@daniel-beck
Copy link
Member

This PR cannot be merged and the author did not respond to a request for a unit test in over a year, so I'm closing this. It could still be reconsidered as a new PR that can be merged and maybe has a test.

(IMO the existence of the warning is appropriate, as polling every minute is often just not a good idea -- the warning just needs to be rephrased to mention that the suggested alternative polls once per hour.)

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.

3 participants