Skip to content
Snippets Groups Projects

Code checks

Merged Code checks
github/fork/kpedro88/Run2_2017 into Run2_2017
Merged Kevin Pedro requested to merge github/fork/kpedro88/Run2_2017 into Run2_2017

CMSSW can run code checks via clang-tidy in scram. The default list is here: https://github.com/cms-sw/cmssw/blob/master/.clang-tidy

I added a few more: cppcoreguidelines-pro-type-member-init, modernize-loop-convert

and then ran over TreeMaker source code as follows:

touch TreeMakerCode.txt
find TreeMaker/ -name "*.h" >> TreeMakerCode.txt
find TreeMaker/ -name "*.cc" >> TreeMakerCode.txt
env USER_CODE_CHECKS_FILE=TreeMakerCode.txt scram b code-checks

The code is automatically updated to conform to the specified code checks.

A few caveats about modernize-loop-convert:

  • doesn't always use const where it could
  • sometimes chooses unclear names for loop variables
  • doesn't seem to recognize range-based loops for edm::View types

I dealt with these few issues by hand.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
TreeMaker/interface/TreeMaker.h
62 62 vector<TreeTypes> VarTypes;
63 63 map<string,unsigned> nameCache;
64 64 // general event information
65 UInt_t runNum;
66 UInt_t lumiBlockNum;
67 ULong64_t evtNum;
65 UInt_t runNum{};
  • Alexx Perloff @aperloff started a thread on commit 40fb97b2
    • Utils/src/METDouble.cc
    142 142 double dpnhat[3];
    143 143 unsigned int goodcount=0;
    144 144
    145 for(int i=0; i<3; ++i)dpnhat[i]=-999;
    145 for(double & i : dpnhat)i=-999;
  • Alexx Perloff @aperloff started a thread on commit 40fb97b2
    • Utils/src/METDouble.cc
    220 220 auto htp5 = std::make_unique<double>(dpnhat[2]);
    221 221 iEvent.put(std::move(htp5),"DeltaPhiN3");
    222 222 float mindpn=9999;
    223 for(int i=0; i<3; ++i){
    224 if(mindpn>fabs(dpnhat[i]))mindpn=fabs(dpnhat[i]);
    223 for(double i : dpnhat){
  • Kevin Pedro
    Kevin Pedro @pedrok started a thread on commit 40fb97b2
    • TreeMaker/interface/TreeMaker.h
    62 62 vector<TreeTypes> VarTypes;
    63 63 map<string,unsigned> nameCache;
    64 64 // general event information
    65 UInt_t runNum;
    66 UInt_t lumiBlockNum;
    67 ULong64_t evtNum;
    65 UInt_t runNum{};
  • Kevin Pedro
    Kevin Pedro @pedrok started a thread on commit 40fb97b2
    • Utils/src/METDouble.cc
    142 142 double dpnhat[3];
    143 143 unsigned int goodcount=0;
    144 144
    145 for(int i=0; i<3; ++i)dpnhat[i]=-999;
    145 for(double & i : dpnhat)i=-999;
  • Alexx Perloff @aperloff started a thread on commit 40fb97b2
    • Utils/src/METDouble.cc
    142 142 double dpnhat[3];
    143 143 unsigned int goodcount=0;
    144 144
    145 for(int i=0; i<3; ++i)dpnhat[i]=-999;
    145 for(double & i : dpnhat)i=-999;
  • A
    Alexx Perloff @aperloff
    Contributor

    Created by: aperloff

    Looks good to me.

  • Kevin Pedro
    Kevin Pedro @pedrok started a thread on commit 40fb97b2
    • Utils/src/METDouble.cc
    142 142 double dpnhat[3];
    143 143 unsigned int goodcount=0;
    144 144
    145 for(int i=0; i<3; ++i)dpnhat[i]=-999;
    145 for(double & i : dpnhat)i=-999;
    • Kevin Pedro
      Kevin Pedro @pedrok
      Author Owner

      The goal of auto is to improve readability. Obviously this is somewhat in the eye of the beholder, but while replacing some 50-character iterator type with auto definitely improves readability, replacing int/double/float etc. probably doesn't.

    Please register or sign in to reply