Skip to content

Follow up on MR !120

Jens Kroeger requested to merge jekroege/corryvreckan:follow-up_on_MR120 into master

This is a merge-request addressing @simonspa's comments on MR !120 (merged).

Here's the list:

Simon:

This merge might have been a bit early, there are several things I would like to see addressed:

  • Hardcoding TLU is not nice, especially with a type-sensitive comparison
  • For adjust_event_times why can't the default not just be empty? Having a default that doesn't mean anything isn't good style imho
  • For require_detector I remember discussing to implement this as getArray<std::string> to directly allow requiring multiple detectors.
  • I don't like the name detector_to_set_track_timestamp and also not that Timepix3_0 is a hard-coded value as default.
  • Track::hasDetector has to be a const function.

Apart from that - nice work!

Edited by Jens Kroeger

Merge request reports