Review of proposed changes in Sti*/ directories for StiCA_2016 branch in STAR CVS
Updated on Thu, 2016-03-03 12:54. Originally created by smirnovd on 2016-03-02 16:11.
Most of the changes (in lines-of-code) do not introduce any new logic or features to the code. Such minor (mainly stylistic) modifications are expected but not guaranteed! to be safe and therefore should not be accepted in production releases until confirmed useful.
All changes are proposed in bulk without explanations. This is not how modern software developement works.
Recommendations:
The changes can be also found on ds-StiCA_2016 branch in the following git repository:
github.com/star-bnl/star-sti/commits/ds-StiCA_2016
Most of the changes (in lines-of-code) do not introduce any new logic or features to the code. Such minor (mainly stylistic) modifications are expected but not guaranteed! to be safe and therefore should not be accepted in production releases until confirmed useful.
All changes are proposed in bulk without explanations. This is not how modern software developement works.
Recommendations:
- There are many changes modifying standard C++ types to their ROOT equivalents, e.g. double -> Double_t - Do not accept
- There are many changes replacing standard C library functions with their equivalents from ROOT, e.g. as fabs() -> TMath::Abs() - Do not accept
- Changes in Sti/StiTPCCATrackerInterface which is not used in standard reconstruction - Need clear log messages explaining the changes - Accept
- Extra debugging print out statements, new commented out code - Do not accept
- Changes removing commented out code - Accept
- Whitespace and style changes - Group in a separate commit with appropriate log message - Accept
- A few changes in StiMaker and StiSstDetectorBuilder reverting recent modifications - Do not accept
- There are few major changes in StiTpcSeedFinder, StiTrackNode, and StiTrackNodeHelper - Need further investigation
- Individual changes need to be considered one by one. Appropriate log messages need to be provided describing the purpose of each modificiation
- In StiTrackNode: /* scale off diagonal elements, fix for gcc 4.8.2 */ - what is that? why?
- In StiTrack: My guess is that the particle mass data member was replaced by a mass based on PDG particle ID. Fine but the change needs a clear description in a separate commit - Accept
- In StiTpcSeedFinder: New logic has somethign to do with hits alignment... "check that all hits are coming from the same sector" - what is that?
- ...
The changes can be also found on ds-StiCA_2016 branch in the following git repository:
github.com/star-bnl/star-sti/commits/ds-StiCA_2016
»
- smirnovd's blog
- Login or register to post comments