Skip to content

Update comments to distinguish between operations and state-storage backends more clearly #37085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SarahFrench
Copy link
Member

Target Release

N/A

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

None.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@SarahFrench SarahFrench added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label May 19, 2025

if !opts.ForceLocal {
log.Printf("[TRACE] Meta.Backend: backend %T does not support operations, so wrapping it in a local backend", b)
}

// Build the local backend
// Build the local operations backend
Copy link
Member Author

Choose a reason for hiding this comment

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

Question for reviewer- I wanted to include this comment but wasn't sure if it added context that couldn't already be learned from the code. Does this comment help with understanding, or does it add clutter?

Suggested change
// Build the local operations backend
// Build the local operations backend
// Note: If b is nil then we are using implicit local state storage.

@SarahFrench SarahFrench marked this pull request as ready for review May 19, 2025 11:18
@SarahFrench SarahFrench requested a review from a team as a code owner May 19, 2025 11:18
// BackendOpts are the options used to initialize a backend.Backend.
// BackendOpts are the options used to initialize a backendrun.OperationsBackend.
Copy link
Member Author

@SarahFrench SarahFrench May 19, 2025

Choose a reason for hiding this comment

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

Similar prompt for reviewer - this change might be debatable.

From my perspective, this struct is used both in logic specific to initialising backendrun.OperationsBackends and backend.Backends, but overall the use of these options is to affect the returned operations backend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog-needed Add this to your PR if the change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant