Skip to content

Remove the getPendingBuildTimes method from base scheduler#2008

Merged
tardyp merged 1 commit intobuildbot:masterfrom
MikeLing:remove-old-scheduler-methods
Mar 25, 2016
Merged

Remove the getPendingBuildTimes method from base scheduler#2008
tardyp merged 1 commit intobuildbot:masterfrom
MikeLing:remove-old-scheduler-methods

Conversation

@MikeLing
Copy link
Copy Markdown
Contributor

This PR is about tickets 2653, which required remove no longer used method from base scheduler. Due to the discussion under the ticket and the comment in here, I just remove the getPendingBuildTimes method :)

@codecov-io
Copy link
Copy Markdown

Current coverage is 83.85%

Merging #2008 into master will not affect coverage as of 230f6dc

@@            master   #2008   diff @@
======================================
  Files          321     321       
  Stmts        31490   31488     -2
  Branches         0       0       
  Methods          0       0       
======================================
- Hit          26405   26403     -2
  Partial          0       0       
  Missed        5085    5085       

Review entire Coverage Diff as of 230f6dc


Uncovered Suggestions

  1. +0.10% via ...ot/status/builder.py#381...414
  2. +0.09% via ...ot/status/builder.py#453...481
  3. +0.08% via ...dbot/changes/mail.py#478...505
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@djmitche
Copy link
Copy Markdown
Member

This is good so far, but I see a lot more places this method is used and defined. Try git grep getPendingBuildTimes (git grep is an awesome tool!)

@tardyp
Copy link
Copy Markdown
Member

tardyp commented Mar 2, 2016

git grep shows a few code more to kill

@tardyp
Copy link
Copy Markdown
Member

tardyp commented Mar 8, 2016

Hi mike. do you want to finish this?

@MikeLing
Copy link
Copy Markdown
Contributor Author

Oh yes, sorry for the late reply! I will commit this PR ASAP!!

@MikeLing
Copy link
Copy Markdown
Contributor Author

@djmitche @tardyp Sorry for the late reply, I somehow miss the notification email until this weekend. Sorry :(

Yeah I know we still have some more getPendingBuildTimes functions, but they are not for the BaseScheduler class. Should we remove them all? It only ask to remove getPendingBuildTimes methods from BaseScheduler in the issue description.

@djmitche
Copy link
Copy Markdown
Member

Yes, please remove all of them -- the request implied all BaseScheduler subclasses, too :)

@MikeLing MikeLing force-pushed the remove-old-scheduler-methods branch 2 times, most recently from babae25 to 6b61ad2 Compare March 14, 2016 03:36
@tardyp
Copy link
Copy Markdown
Member

tardyp commented Mar 17, 2016

👍 keeping it a bit for others to review

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.

4 participants