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

Calibcalorimetry packages: clang warning cleanup #22425

Merged
merged 6 commits into from Mar 5, 2018
Merged

Calibcalorimetry packages: clang warning cleanup #22425

merged 6 commits into from Mar 5, 2018

Conversation

gartung
Copy link
Member

@gartung gartung commented Mar 2, 2018

No description provided.

@gartung
Fix clang warning by setting ic = NHBHECR which is the value it shoul…
81055d3 
…d be after last for loop and ic++

CalibCalorimetry/HcalAlgos/src/HcalLogicalMapGenerator.cc:1665:9: warning: assigning field to itself [-Wself-assign-field]
@gartung
Fix clang warning
ef5abfa 
CalibCalorimetry/EcalTrivialCondModules/src/EcalTrivialConditionRetriever.cc:1104:8: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
@gartung
Fix clang warnings:
06acc5b 
CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/src/TEcnaNArrayD.cc:359:1: warning: all paths through this function will call itself [-Winfinite-recursion]
CalibCalorimetry/EcalPedestalOffsets/src/testChannel.cc:28:67: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int]
@gartung
Change fix for clang warning:
1a0538a 
CalibCalorimetry/HcalAlgos/src/HcalLogicalMapGenerator.cc:1665:9: warning: assigning field to itself [-Wself-assign-field]
@cmsbuild cmsbuild added this to the CMSSW_10_1_X milestone Mar 2, 2018
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

The code-checks are being triggered in jenkins.

@gartung
Copy link
Member Author

gartung commented Mar 2, 2018

@Dr15Jones

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22425/3680

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22425/3680/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22425/3680/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22425/3681

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

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

It involves the following packages:

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos
CalibCalorimetry/EcalPedestalOffsets
CalibCalorimetry/EcalTrivialCondModules
CalibCalorimetry/HcalAlgos

@ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @mariadalfonso, @mmusich, @tocheng, @argiro this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

Dr15Jones
Dr15Jones reviewed Mar 2, 2018
View reviewed changes
CalibCalorimetry/HcalAlgos/src/HcalLogicalMapGenerator.cc Outdated
@@ -1659,10 +1659,10 @@ void HcalLogicalMapGenerator::buildCALIBMap(const HcalTopology* topo,
}
}
}
ic++;
//ic++; // replace with ic=ic+1 below
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the comment since once the code is changed, there is no need for it

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22425/3683

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

Pull request #22425 was updated. @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please check and sign again.

@Dr15Jones
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26457/console Started: 2018/03/02 18:41

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

+1
Tested at: 8aa0fa2
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22425/26457/summary.html

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 29
  • DQMHistoTests: Total histograms compared: 2479021
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2478844
  • DQMHistoTests: Total skipped: 176
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.00000000008 KiB( 22 files compared)
  • Checked 118 log files, 9 edm output root files, 29 DQM output files

@lpernie
Copy link
Contributor

lpernie commented Mar 5, 2018

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2018

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

fabiocos commented Mar 5, 2018

+1

@cmsbuild cmsbuild merged commit b298817 into cms-sw:master Mar 5, 2018
@gartung gartung deleted the CalibCalorimetry-clang-warning-cleanup branch March 6, 2018 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dr15Jones Dr15Jones Dr15Jones left review comments

Assignees
No one assigned
Labels
alca-approved code-checks-approved comparison-available fully-signed orp-approved tests-approved
Projects
None yet
Milestone
CMSSW_10_1_X
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@gartung @cmsbuild @Dr15Jones @lpernie @fabiocos