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-39498 Fix invalid data #335

Merged
merged 3 commits into from Dec 6, 2017

Conversation

Projects
None yet
2 participants
@scoheb
Copy link
Contributor

commented Nov 20, 2017

No description provided.

scoheb added some commits Nov 20, 2017

@rsandell
Copy link
Member

left a comment

Thanks! looks OK. Waiting with merge until Sergii comes back with results on the ticket.

@@ -155,6 +158,38 @@ protected synchronized Date getDateFromTimestamp() {
return new Date();
}

@Override
public void run() {

This comment has been minimized.

Copy link
@rsandell

rsandell Dec 1, 2017

Member

Probably better implemented as an AsyncPeriodicWork to save on thread count.

@scoheb

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2017

@rsandell Sergii's tests were successful. Would you still like me to refactor to use AsyncPeriodicWork before we merge?

@rsandell

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

@scoheb It's up to you. I can merge as is, and we can get that nit fixed sometime down the line.

@scoheb scoheb force-pushed the scoheb:missed-events branch from 0fe724a to 2d161a2 Dec 5, 2017

@scoheb scoheb force-pushed the scoheb:missed-events branch from 2d161a2 to 9242b6a Dec 5, 2017

@scoheb

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2017

@rsandell refactoring complete. Take a look at your convenience.

@rsandell rsandell merged commit fde68b3 into jenkinsci:master Dec 6, 2017

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
@rsandell

This comment has been minimized.

Copy link
Member

commented Dec 6, 2017

Thank you very much @scoheb for looking into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.