Rename timeout watchdog so file name/module name/cmake name all agree

[michaelmackenzie]

There were errors in the spack build test, where hopefully this renaming solves these errors.

[FNALbuild]

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

• CalPatRec
• TimeoutService

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

[FNALbuild]

☀️ The build tests passed at e84ae55.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged e84ae55 at 68688bb
build (prof)	✅	Log file. Build time: 04 min 05 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 (1) in 5 files
clang-tidy	➡️	7 errors 209 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at e84ae55 after being merged into the base branch at 68688bb.

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]

I'll review this pull request by examining the details and the changes made.Now let me check the full files for some context around the changes and look for any remaining references to the old name:Now let me also check for any other references to the old name that might have been missed:

Here's my review of PR #1793. The rename from TimeoutWatchdogService → TimeoutWatchdog is straightforward and the core changes look correct. However, I found a few issues:

---

1. 🔴 Stale comment in TimeoutWatchdog_service.cc

//
// TimeoutWatchdogService plugin
//

The comment on line 2 still says TimeoutWatchdogService plugin — it should be updated to TimeoutWatchdog plugin (or similar) to match the rename.

---

2. 🟡 Misaligned indentation in TimeoutWatchdog.cc constructor

TimeoutWatchdog::TimeoutWatchdog(Parameters const& config,
                                               art::ActivityRegistry& registry)

The second line's indentation still matches the old longer name (TimeoutWatchdogService::TimeoutWatchdogService). The art::ActivityRegistry& registry parameter is now over-indented relative to the opening parenthesis. This is cosmetic but worth fixing for consistency.

---

3. 🟡 Same indentation issue in startModule

void TimeoutWatchdog::startModule(std::string const& moduleLabel,
                                         std::optional<double> allowedTimeMs) {

The second parameter is over-indented for the same reason — it was previously aligned to TimeoutWatchdogService::startModule(.

---

4. 🟠 Unresolved FIXME comment (pre-existing but worth noting)

  if (tls_.stopToken.stop_requested()) { //FIXME: Need to decide if a previous module was timed out but the event wasn't, do we allow next modules to continue anyway

This is pre-existing, but since the file is being touched anyway, it might be a good time to address or at least track this as an issue.

---

5. ✅ Completeness check — all references updated

The code search for TimeoutWatchdogService on the PR's head branch returned no results, confirming that all references to the old name have been properly renamed across:

• TimeoutService/inc/TimeoutWatchdog.hh — class, constructors, DECLARE_ART_SERVICE
• TimeoutService/src/TimeoutWatchdog.cc — all method definitions and debug strings
• TimeoutService/src/TimeoutWatchdog_service.cc — DEFINE_ART_SERVICE
• TimeoutService/CMakeLists.txt — cet_build_plugin
• CalPatRec/src/AgnosticHelixFinder_module.cc — ServiceHandle and ModuleGuard usage
• CalPatRec/src/TZClusterFinder_module.cc — ServiceHandle and ModuleGuard usage

---

Summary

Severity	Issue	File
🔴 Bug	Stale TimeoutWatchdogService in comment	TimeoutWatchdog_service.cc:2
🟡 Style	Over-indented constructor parameter	TimeoutWatchdog.cc:11
🟡 Style	Over-indented startModule parameter	TimeoutWatchdog.cc:61
🟠 Note	Pre-existing FIXME on stop-token propagation	TimeoutWatchdog.cc:64

Overall this is a clean, well-scoped rename. The only actual correctness issue is the stale comment in the service definition file; the rest are cosmetic. No functional or logic bugs found.

[FNALbuild]

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

[michaelmackenzie]

I adjusted the spacing in the file and updated the comment. The FIXME is intentional it it'll require input and discussions at future trigger meetings to decide our approach on these events.

[oksuzian]

@FNALbuild run build test

[FNALbuild]

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

[FNALbuild]

☀️ The build tests passed at 3f4c7fa.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged 3f4c7fa at 2cc428c
build (prof)	✅	Log file. Build time: 04 min 16 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. Return Code 2.
check_cmake	✅	Log file.
FIXME, TODO	➡️	TODO (0) FIXME (1) in 5 files
clang-tidy	➡️	7 errors 209 warnings
whitespace check	✅	no whitespace errors found

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

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.