This code needs to be in 'common_cpp' not legacy. You should be able to just move it.

New code is forbidden within legacy.

https://code.launchpad.net/~e-santos10/maus/devel

Will introduce the required changes in the next submission. Thank you!

DoubletFiberParam.cc

1) cpplint: 18 errors

2) This code is basically all geometry e.g.

G4double spacing = sqrt(fiberDiameter*fiberDiameter*
                         (1-fiberPitch*fiberPitch/4));

the finer details of which are beyond me...are all these formulae tested/verified somehow?

SciFiSD.cc

1) cpplint: 62 errors, spacing again!

2) I think more explanation is necessary to describe what ProcessHits does. Are you reading in hit information/channel for each G4step through the SciFiPlanes?

Users may have little understanding of how these executables fit into the bigger picture of the Tracker Code as a whole, so you could try to put it in context - or include a link showing the overall structure/design of the Tracker software as a whole.

3) If I understand correctly, old_chanNo is only required for comparison with chanNo, which are both output to debug.txt. So is legacy_chanNo() a fully-fledged test now or genuine legacy code to be phased out [and replaced?]?

The unit tests are being improved. Parameters like the one you are showing are compared with the values presented/derived from MICE note 135:

http://mice.iit.edu/micenotes/public/pdf/MICE0135/MICE0135.pdf

ES

Are you wanting to merge with the trunk? Or are you just wanting input?

This relates to issues:

512 unit tests
507 refactor
506 code review
505 style guide and comments
504 (parent) cleanup scifiSD

Is this all of those issues? I also don't see SciFiSD in your branch anywhere....

http://bazaar.launchpad.net/~e-santos10/maus/devel/files/head:/src

Am I missing something?

Sorry, I moved it like you said and forgot to "add" the new folder to bzr. Will do it now.

I'll have to see which issues are related with this, but it's a bunch of them. I'll look tomorrow.

Tunnell, Christopher wrote:

Are you wanting to merge with the trunk? Or are you just wanting input?

This relates to issues:

512 unit tests
507 refactor
506 code review
505 style guide and comments
504 (parent) cleanup scifiSD

Is this all of those issues? I also don't see SciFiSD in your branch anywhere....

http://bazaar.launchpad.net/~e-santos10/maus/devel/files/head:/src

Am I missing something?

Chris, this relates to issues

335 - tracker geometry issue when finding fibre number (affects physics resolutions)
504 - Clean up SciFiSD
505 - SciFiSD style guide and comments
506 - SciFiSD code review
507 - SciFiSD refactor
509 - Tracker Geometry
512 - SciFiSD unit tests !!!!!!!!!!!WHICH WILL REMAIN OPEN!!!!!!!!!!!!!!!!!!!!!!!
595 - Code review - DetModel/SciFi/

Tunnell, Christopher wrote:

Are you wanting to merge with the trunk? Or are you just wanting input?

This relates to issues:

512 unit tests
507 refactor
506 code review
505 style guide and comments
504 (parent) cleanup scifiSD

Is this all of those issues? I also don't see SciFiSD in your branch anywhere....

http://bazaar.launchpad.net/~e-santos10/maus/devel/files/head:/src

Am I missing something?

So it's without tests? Is that what you mean when saying that that issue stays open?

We can say the code looks good but it can't go into the trunk without tests. It would be nice to finish at least some of the tracker stuff.

Can you commit on a date that the unit tests will be done? It really should be done this month for the SciFiSD stuff. Including time to expect a week of corrections.

I can't compile. Where'd the files at src/input/InputCppData/ go?

Can you do:

bzr branch ~e-santos10/maus/devel maus-test
cd maus-test
./tests/integration/install_then_build_then_test.bash

and make sure it works? You should also do:

bzr merge lp:maus

to sync with the trunk first. Then make sure your branch works here:

http://test.mice.rl.ac.uk/job/MAUS_santos/

which will get rebuilt every time you commit (within 24 hours)

scons -c; scons

doesn't solve it either - tried before.

The trunk builds from scratch on a wide range of machines so something must have changed. Can you see what's different versus the trunk?

I can't see what's wrong. I think I'm going to pull a new installation from the trunk and take the files I changed there.

If you create a launchpad branch of just the scifiSD stuff then that greatly simplifies including it into the trunk. Smaller but frequent chunks of code being submitted makes everybody's life easier.

With Chris Heidt's comments added in (and verified), is this tracker code ready to go into the trunk? I know you just have it put down as 'code review' but is this code finished? It would be nice to cross things off the list.