Skip to content

Remove obsolete limit_by_batch_size_flag from dontime - #2447

Open
bolekk wants to merge 1 commit into
mainfrom
remove-limit-by-batch-size-flag
Open

bolekk wants to merge 1 commit into
mainfrom
remove-limit-by-batch-size-flag

Conversation

@bolekk

@bolekk bolekk commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • The limit_by_batch_size_flag field on dontime.Observation was always set to true and never read anywhere — Outcome() already trims ObservedDonTimes to batch size unconditionally.
  • Removes the Go field usage, reserves field 4 in the proto (field 3 was already reserved), and regenerates dontime.pb.go.

Deployment Validation

  • Outcome doesn't use the flag in all envs. Confirm no non-determinism warnings during rollout.

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ API Diff Results - github.com/smartcontractkit/chainlink-common

⚠️ Breaking Changes (2)

pkg/workflows/dontime/pb.(*Observation) (1)
  • GetLimitByBatchSizeFlag — 🗑️ Removed
pkg/workflows/dontime/pb.Observation (1)
  • LimitByBatchSizeFlag — 🗑️ Removed

📄 View full apidiff report

The flag was always set to true and never read; batch-size trimming
in Outcome() is now unconditional.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bolekk
bolekk force-pushed the remove-limit-by-batch-size-flag branch from 1cbba88 to 93a4250 Compare October 7, 2026 05:08
@bolekk
bolekk requested a review from jmank88 October 7, 2026 05:10
@bolekk
bolekk marked this pull request as ready for review October 7, 2026 05:30
@bolekk
bolekk requested a review from a team as a code owner October 7, 2026 05:30
map<string, int64> requests = 2;
reserved 3;
bool limit_by_batch_size_flag = 4;
reserved 3, 4;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit/ is this more desirable than using [deprecated = true]? Because deprecation would be a bit safer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's no need to leave the old names around if nobody uses them. What makes it safer?

This branch has not been deployed

No deployments
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