Project

General

Profile

Support #596

Code review - MapCppTrackerDigitization

Added by Santos, Edward over 12 years ago. Updated about 12 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Santos, Edward
Category:
Tracker
Target version:
Future MAUS release
Start date:
22 July 2011
Due date:
05 August 2011
% Done:

100%

Estimated time:
Workflow:

Description

For code in src/map/MapCppTrackerDigitization/.

#1

Updated by Rogers, Chris over 12 years ago

  • Due date set to 05 August 2011
  • Category set to Tracker
  • Assignee set to Carlisle, Timothy
  • Target version set to Future MAUS release

Assigned to Tim. Note the code review guidelines at:

http://micewww.pp.rl.ac.uk/projects/maus/wiki/Code_review

Get the code with command like:

bzr branch lp:~e-santos10/maus/devel

#3

Updated by Santos, Edward over 12 years ago

  • Subject changed from Code review request to Code review - MapCppTrackerDigitization
#4

Updated by Carlisle, Timothy over 12 years ago

  • Assignee changed from Carlisle, Timothy to Santos, Edward

Looks pretty good to me!

1) cpplint flagged up ~100 very minor errors: white space, spacing etc, overlong lines which could easily be tidied up, depending how rigidly we endorse cpplint..!

2) ADC / TDC aren't explained, so a brief outline would be enlightening for new members to MICE. Also adding them to: http://micewww.pp.rl.ac.uk/projects/mice/wiki/Lingo would be good too.

3) Why group in sets of 7 fibers? 7 fibers = 1 channel right? A short justification of this would save the user a trip to the TRD - I had to double check as I'd forgotten!

4) L117 SP in the comment: "enxct entry"

5) L191 a comment just to explicitly state what you're doing here with the time resolution would be nice. Same for the SciFidcFactor, as it doesn't mean very much to me.

6) L195 Could you add another line explaining what this means? I'm not well versed in the detector stuff, so a little more information would be helpful.

Thanks

#5

Updated by Rogers, Chris over 12 years ago

We stick to the letter of cpplint I'm afraid. It's a pain, but it's the only way to make it work. can make exceptions by appending to

tests/style/cpplint_exceptions.py

#6

Updated by Rogers, Chris about 12 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

I think this is closed now.

Also available in: Atom PDF