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

RecoTracker/DebugTools: bug fix beginRun not called because of incorrect parameters #16976

Merged
merged 2 commits into from Dec 21, 2016
Merged

RecoTracker/DebugTools: bug fix beginRun not called because of incorrect parameters #16976

merged 2 commits into from Dec 21, 2016

Conversation

gartung
Copy link
Member

@gartung gartung commented Dec 12, 2016

This fixes clang warning hides overloaded virtual function [-Woverloaded-virtual]by adding const to function declaration of beginRun. A potential bug fix because beginRun is now called where it was not before.

…nction [-Woverloaded-virtual] by adding const to function declaration of beginRun.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gartung (Patrick Gartung) for CMSSW_9_0_X.

It involves the following packages:

RecoTracker/DebugTools

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @gpetruc, @dgulhan this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@cvuosalo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 12, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16974/console Started: 2016/12/12 21:08

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

@rovere, @vincenzo: Are the contents of the RecoTracker/DebugTools even used anymore? Maybe we could just delete it rather than updating it?

@cvuosalo
Copy link
Contributor

@gartung: If we don't delete the directory, please add override to ensure this problem doesn't happen silently again when the base signature changes.

@vincenzo
Copy link

@cvuosalo I think you got the wrong Vincenzo there. Kinda feel you wanted to tag @VinInn ;)

@cvuosalo
Copy link
Contributor

@VinInn: Are the contents of the RecoTracker/DebugTools even used anymore? Maybe we could just delete it rather than updating it?

@cvuosalo
Copy link
Contributor

@gartung: We might as well good ahead with this PR. Maybe some day in the future the whole directory can be deleted.
Please add override specifiers just as you've done in the other PRs.

@cmsbuild
Copy link
Contributor

Pull request #16976 was updated. @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please check and sign again.

@cvuosalo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 20, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17127/console Started: 2016/12/20 17:25

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

+1

For #16976 33b5f12

Fixing Clang warnings about overloading in RecoTracker/DebugTools. It is uncertain whether this directory is even used at all anymore. There should be no change in standard workflows.

In the future, we can consider deleting this directory entirely if the tracking group (and everyone else) doesn't object.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_9_0_X_2016-12-19-2300 show no significant differences, as expected.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 980f2d4 into cms-sw:CMSSW_9_0_X Dec 21, 2016
@gartung gartung deleted the RecoTracker-DebugTools-fix-clang-warnings branch December 21, 2016 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants