Skip to content

Remove some obsolete code#1738

Closed
brownd1978 wants to merge 1 commit intoMu2e:mainfrom
brownd1978:obsolete
Closed

Remove some obsolete code#1738
brownd1978 wants to merge 1 commit intoMu2e:mainfrom
brownd1978:obsolete

Conversation

@brownd1978
Copy link
Collaborator

@brownd1978 brownd1978 commented Feb 18, 2026

I have contacted the authors privately and they agree these files can be removed.

@FNALbuild
Copy link
Collaborator

Hi @brownd1978,
You have proposed changes to files in these packages:

  • /
  • Analyses
  • TEveEventDisplay
  • Filters

which require these tests: build.

@Mu2e/fnalbuild-users, @Mu2e/write have access to CI actions on main.

⌛ The following tests have been triggered for d4c948b: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at d4c948b.

Test Result Details
test with Command did not list any other PRs to include
merge Merged d4c948b at 2fed5e8
build (prof) Log file. Build time: 04 min 09 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
trigger Log file.
check_cmake Log file.
FIXME, TODO TODO (0) FIXME (0) in 24 files
clang-tidy 0 errors 0 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at d4c948b after being merged into the base branch at 2fed5e8.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@oksuzian
Copy link
Collaborator

I'll review this pull request by first loading the relevant details and the code changes.Now let me check if there are any remaining references to the removed code elsewhere in the codebase:Here's my review of PR #1738 — "Remove some obsolete code" by @brownd1978:

PR Overview

This is a large deletion PR: +1 / -101,050 across 67 changed files in a single commit. It removes two main categories of code:

  1. TrackSummary-related analyzer/filter modules (from Analyses/ and Filters/)
  2. The entire TEveEventDisplay package (a ROOT TEve-based event display)

The build CI has passed (label: build finished), and the mergeable state is clean.


Issues Found

1. 🔴 Orphaned Data Products: TrackSummary and TrackSummaryTruthAssns remain in the codebase

The PR removes the consumers of TrackSummary and TrackSummaryTruthAssns (the analyzer/filter modules), but leaves behind the data product definitions themselves:

  • RecoDataProducts/inc/TrackSummary.hh and RecoDataProducts/src/TrackSummary.cc — still compiled in RecoDataProducts/CMakeLists.txt
  • MCDataProducts/inc/TrackSummaryTruthAssns.hh and MCDataProducts/src/TrackSummaryTruthAssns.cc — still compiled in MCDataProducts/CMakeLists.txt
  • The dictionary entries in MCDataProducts/src/classes_def.xml and MCDataProducts/src/classes.h still reference TrackSummaryMatchInfo, TrackSummaryTruthAssns, etc.
  • The dictionary entries in RecoDataProducts/src/classes_def.xml and RecoDataProducts/src/classes.h likely still reference TrackSummary.
  • Related utility classes HistTrackSum and TrackCuts in Mu2eUtilities (referenced by the deleted analyzers) may also be orphaned now.

Impact: If these data products are truly obsolete (no other code produces or consumes them), they should be removed too. If they're left behind, they add dead code and maintenance burden. However, if other code still uses them (e.g., a TrackSummaryTruthMaker producer module), removing only the consumers is fine. This should be clarified.

2. 🟡 Orphaned Shell Script: bin/TEveMu2e.sh

The file bin/TEveMu2e.sh directly references TEveEventDisplay/fcl/prolog.fcl and is installed via bin/CMakeLists.txt:

  TEveMu2e.sh

This script is not removed in the PR, but it will be completely broken since the entire TEveEventDisplay/ directory (including fcl/prolog.fcl) is deleted. It should be removed along with the package.

3. 🟡 Minor: Extra Blank Line in Analyses/CMakeLists.txt

After removing the three TrackSummary* build targets, the diff shows a double blank line left behind before the TSTrackAna entry. This is cosmetic but slightly inconsistent with the rest of the file.

4. 🟢 Positive: Trailing Whitespace Cleanup in Filters/CMakeLists.txt

The PR cleans up trailing whitespace on the CompressStepPointMCs LIBRARIES REG line — a good incidental fix.

5. ⚠️ Note: Incomplete File List (API limitation)

The API returned only 30 of the 67 changed files. There may be additional issues in the remaining files (likely more TEveEventDisplay/ source files being deleted). You can view the full file list on GitHub.


Summary

Severity Issue Action Needed
🔴 High TrackSummary / TrackSummaryTruthAssns data products and utilities remain but may now be orphaned Confirm if still used; if not, remove them too
🟡 Medium bin/TEveMu2e.sh not deleted — will be broken Remove the script and its CMakeLists.txt entry
🟡 Low Extra blank line in Analyses/CMakeLists.txt Clean up
🟢 Good Trailing whitespace fix in Filters/CMakeLists.txt No action

The most important item to resolve is whether TrackSummary and related data products are used anywhere else in the codebase. If not, this PR should either include their removal or a follow-up PR should be planned. The bin/TEveMu2e.sh script should definitely be removed as part of this PR since the TEveEventDisplay package it depends on is being entirely deleted.

@FNALbuild
Copy link
Collaborator

📝 The HEAD of main has changed to 3e5e0e6. Tests are now out of date.

@oksuzian
Copy link
Collaborator

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

⌛ The following tests have been triggered for d4c948b: build (Build queue - API unavailable)

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at d4c948b.

Test Result Details
test with Command did not list any other PRs to include
merge Merged d4c948b at 3e5e0e6
build (prof) Log file. Build time: 04 min 09 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
trigger Log file.
check_cmake Log file.
FIXME, TODO TODO (0) FIXME (0) in 24 files
clang-tidy 0 errors 0 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at d4c948b after being merged into the base branch at 3e5e0e6.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@brownd1978
Copy link
Collaborator Author

Subsumed by #1742

@brownd1978 brownd1978 closed this Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants