Skip to content

fix: typo AggSender.TriggerASAP.DelayBeetweenCertificates#1471

Merged
joanestebanr merged 3 commits intodevelopfrom
fix/typo-cherry-pick
Feb 5, 2026
Merged

fix: typo AggSender.TriggerASAP.DelayBeetweenCertificates#1471
joanestebanr merged 3 commits intodevelopfrom
fix/typo-cherry-pick

Conversation

@joanestebanr
Copy link
Collaborator

@joanestebanr joanestebanr commented Feb 5, 2026

🔄 Changes Summary

  • Fix typo: AggSender.TriggerASAP.DelayBeetweenCertificates

⚠️ Breaking Changes

  • Deprecated field AggSender.TriggerASAP.DelayBeetweenCertificates in favour of DelayBetweenCertificates

📋 Config Updates

  • 🧾 Diff/Config snippet:
[AggSender.TriggerASAP]
  DelayBetweenCertificates = "1s"

🔗 Related PRs

@joanestebanr joanestebanr marked this pull request as ready for review February 5, 2026 10:11
@claude
Copy link

claude bot commented Feb 5, 2026

Claude finished @joanestebanr's task in 1m 17s —— View job


Code Review Complete ✅

Tasks

  • Read all changed files to understand the scope
  • Verify the typo fix is complete and consistent
  • Check for any missed references
  • Provide comprehensive review feedback
  • Fill in PR summary template

🚀 What's New

This PR fixes a typo in the configuration field name DelayBeetweenCertificatesDelayBetweenCertificates across the codebase. The typo is corrected in:

  • Struct field definitions and mapstructure tags
  • Default configuration values
  • String formatting methods
  • Validation error messages
  • Code references in trigger implementation
  • Unit test assertions
  • Documentation

🐛 Bug Fixes

  • Fixed typo: DelayBeetweenCertificatesDelayBetweenCertificates in TriggerASAPConfig

📋 Config Updates

Field Name Change:

[AggSender.TriggerASAP]
    DelayBetweenCertificates = "1s"  # Previously: DelayBeetweenCertificates

Default value remains: 1s

⚠️ Note: This is a configuration field rename. Users with existing config files using the old field name (DelayBeetweenCertificates) will need to update their configurations.

⚠️ Breaking Changes

Configuration Breaking Change: The field name in TriggerASAPConfig has changed from DelayBeetweenCertificates to DelayBetweenCertificates. Existing configuration files must be updated to use the corrected field name.


Review Feedback

🟢 Overall Assessment

This is a clean, straightforward typo fix that has been applied consistently across all relevant files. The changes are well-contained and correctly update:

  • Go struct definitions (aggsender/config/config.go:22)
  • Struct methods (aggsender/config/config.go:31, 37, 44)
  • Runtime code (aggsender/trigger/trigger_asap.go:147, 158)
  • Unit tests (aggsender/trigger/trigger_asap_test.go:78)
  • Default configuration (config/default.go:223)
  • Documentation (docs/aggsender.md:230, 238, 242)

✅ Positive Observations

  1. Complete coverage: All instances of the typo have been corrected
  2. Consistency: Field name, mapstructure tag, String() output, Validate() error message, and documentation are all aligned
  3. Test coverage: Unit test assertion has been updated to match the corrected field name
  4. No logic changes: The fix is purely cosmetic (correcting the spelling)

🟡 Considerations

1. Breaking Change Classification
While this is technically a breaking change for configuration files, it's a necessary correction. Consider:

  • Adding a note in the release notes/changelog about this config field rename
  • Potentially providing a deprecation warning for the old field name (though this may be overkill for a typo fix)
  • Updating any example configs or deployment scripts that may reference the old field name

2. Related Typo
I noticed there's a similar typo in a different field (DelayBeetweenRetries in config/config.go:68, 186 and config/config_test.go:150), though it appears to be deprecated. While not strictly part of this PR's scope, you might want to track fixing that typo as well if the field is still in use.

3. mapstructure Tag
The mapstructure tag has been correctly updated (aggsender/config/config.go:22), which means the configuration parser will now look for DelayBetweenCertificates instead of DelayBeetweenCertificates in config files. This is the primary breaking change point.

🟢 Code Quality

  • Go formatting appears correct
  • No security concerns
  • No performance implications
  • Changes follow the existing code style
  • Documentation is clear and accurate

Verification Notes

  • All 5 changed files have been reviewed
  • Searched codebase for remaining instances of Beetween - found only unrelated DelayBeetweenRetries (different field, appears deprecated)
  • The fix is complete and consistent across code, tests, config, and docs

Recommendation

✅ APPROVED - This is a clean typo fix with appropriate coverage. The breaking change is justified as it corrects a spelling error in a configuration field name.


@joanestebanr joanestebanr self-assigned this Feb 5, 2026
@joanestebanr joanestebanr changed the title Fix: typo AggSender.TriggerASAP.DelayBeetweenCertificates fix: typo AggSender.TriggerASAP.DelayBeetweenCertificates Feb 5, 2026
jhkimqd and others added 3 commits February 5, 2026 12:19
Signed-off-by: Ji Hwan <jkim@polygon.technology>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 5, 2026

@joanestebanr joanestebanr merged commit 0350201 into develop Feb 5, 2026
22 checks passed
@joanestebanr joanestebanr deleted the fix/typo-cherry-pick branch February 5, 2026 14:24
joanestebanr added a commit that referenced this pull request Feb 5, 2026
## 🔄 Changes Summary
- Fix typo: `AggSender.TriggerASAP.DelayBeetweenCertificates`

## ⚠️ Breaking Changes
- Deprecated field ` `AggSender.TriggerASAP.DelayBeetweenCertificates`
in favour of `DelayBetweenCertificates`
    DelayBetweenCertificates = "1s"
```
[AggSender.TriggerASAP]

```

## 📋 Config Updates
- 🧾 **Diff/Config snippet**: [Optional: Enumerate config changes/provide
snippet of changes]

## 🔗 Related PRs
- #1470

---------

Signed-off-by: Ji Hwan <jkim@polygon.technology>
Co-authored-by: Ji Hwan <jkim@polygon.technology>
joanestebanr added a commit that referenced this pull request Feb 5, 2026
Cherry-pick from `develop` #1471

## 🔄 Changes Summary
- Fix typo: `AggSender.TriggerASAP.DelayBeetweenCertificates`

## ⚠️ Breaking Changes
- Deprecated field ` `AggSender.TriggerASAP.DelayBeetweenCertificates`
in favour of `DelayBetweenCertificates`
    DelayBetweenCertificates = "1s"
```
[AggSender.TriggerASAP]

```

## 📋 Config Updates
- 🧾 **Diff/Config snippet**: [Optional: Enumerate config changes/provide
snippet of changes]

## 🔗 Related PRs
- #1470

Signed-off-by: Ji Hwan <jkim@polygon.technology>
Co-authored-by: Ji Hwan <jkim@polygon.technology>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants