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

Jump to bottom

Fix clang warnings in Calibration/IsolatedParticles. #3747

Merged
merged 1 commit into from May 9, 2014
Merged

Fix clang warnings in Calibration/IsolatedParticles. #3747

merged 1 commit into from May 9, 2014

Conversation

ghost
Copy link

@ghost ghost commented May 8, 2014

Dumped unused variables.

@ghost ghost added alca-pending labels May 8, 2014
@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2014

A new Pull Request was created by @Degano (Alessandro Degano) for CMSSW_7_1_X.

Fix clang warnings in Calibration/IsolatedParticles.

It involves the following packages:

Calibration/IsolatedParticles

@cmsbuild, @Degano, @diguida, @rcastello, @nclopezo can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

diguida
diguida reviewed May 8, 2014
View reviewed changes
Calibration/IsolatedParticles/plugins/IsolatedGenParticles.h
int myverbose_;
bool useJetTrigger_;
double drLeadJetVeto_, ptMinLeadJet_;
int verbosity;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused data members...

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2014

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3747/1380/summary.html

@ghost ghost added alca-pending labels May 8, 2014
@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2014

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3747/1380/summary.html

@ghost ghost added alca-pending labels May 8, 2014
diguida
diguida reviewed May 8, 2014
View reviewed changes
Calibration/IsolatedParticles/plugins/IsolatedTracksNxN.h
/// number of DAQ partitions
unsigned int m_numberDaqPartitions;
/// trigger masks & veto masks
const L1GtTriggerMask* m_l1GtTmAlgo;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused data members...

@diguida
Copy link
Contributor

diguida commented May 8, 2014

+1
This PR cleans up unused data members in IsolatedGenParticles, IsolatedTracksCone, and IsolatedTracksNxN, and fixes an unsigned vs signed comparison.

diguida
diguida reviewed May 8, 2014
View reviewed changes
Calibration/IsolatedParticles/interface/CaloSimInfo.icc
@@ -157,7 +157,7 @@ namespace spr{
std::vector< typename T::const_iterator> matchedhit;
for (int ihit=0; ihit < hit.size(); ihit++) {
bool keephit = false;
for (int idetid=0; idetid < coneRecHitDetIds.size(); idetid++) {
for (unsigned int idetid=0; idetid < coneRecHitDetIds.size(); idetid++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsigned vs signed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should actually be size_t, no? (Agreed if we have more than 4G coneRecHitDetIds we have other problems, but...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktf I agree.
Sorry, I missed the issue given by the different size of unsigned int and size_t on 64 bits...
Do we wait for @Degano to update it?

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

@ghost ghost added alca-approved labels May 8, 2014
@ghost
Copy link
Author

ghost commented May 9, 2014

I updated the PR with the correct type now.

@ghost ghost added alca-pending labels May 9, 2014
@cmsbuild
Copy link
Contributor

cmsbuild commented May 9, 2014

Pull request #3747 was updated. @cmsbuild, @Degano, @diguida, @rcastello, @nclopezo can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 9, 2014

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3747/18/summary.html

@ghost ghost added alca-pending labels May 9, 2014
@diguida
Copy link
Contributor

diguida commented May 9, 2014

+1
This PR cleans up unused data members in IsolatedGenParticles, IsolatedTracksCone, and IsolatedTracksNxN, and fixes an unsigned vs signed comparison with the correct size_t type.
Thanks @Degano for reminding me type sizes ;-)

@cmsbuild
Copy link
Contributor

cmsbuild commented May 9, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

@ghost ghost added alca-approved labels May 9, 2014
ktf added a commit that referenced this pull request May 9, 2014
@ktf
Merge pull request #3747 from degano/fix_warn_clang_isolatedparticles…
3137b58 
…_71X

Clang -- Fix clang warnings in Calibration/IsolatedParticles.
@ktf ktf merged commit 3137b58 into cms-sw:CMSSW_7_1_X May 9, 2014
@ghost ghost deleted the fix_warn_clang_isolatedparticles_71X branch May 9, 2014 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
alca-approved comparison-pending fully-signed tests-approved
Projects
None yet
Milestone
CMSSW_7_1_0_pre8
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cmsbuild @diguida @ktf @aledegano