WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@trevorchaney
Copy link
Contributor

@trevorchaney trevorchaney commented Aug 30, 2025

This PR consolidates error field synchronization by adding error setting methods that ensures i.Err and i.werr are always kept in sync.

Changes

  • Added 'initErrors' and 'seterrors' method to Info struct that takes an error pointer and updates both error fields
  • Added nil pointer assertion with panic for fail-fast behavior on programming errors
  • Replaced manual error field assignments throughout the codebase with calls to initErrors and setErrors

Benefits

  • Eliminates manual error synchronization patterns that could lead to inconsistencies
  • Provides fail-fast behavior for programming errors (nil pointer detection)
  • Improves code maintainability and reduces duplication
  • All existing functionality preserved with comprehensive test coverage

Testing

  • All unit tests pass
  • Race detection tests pass (verified with WSL)
  • Performance benchmarks show no regressions
  • Comprehensive test suite validates no breaking changes

@lainio lainio self-requested a review September 1, 2025 15:51
@lainio
Copy link
Owner

lainio commented Sep 1, 2025

Thank you! I like this. I added a comment about the name of the function. Let's start the discussion 😃

@trevorchaney trevorchaney changed the title Refactor error field synchronization with updateBothErrors helper Refactor error field synchronization with helper methods Sep 1, 2025
Copy link
Owner

@lainio lainio left a comment

Choose a reason for hiding this comment

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

Works for me, and the test matrix is green. I also ran make inline_hander for checks.

@lainio lainio merged commit d12b1d3 into lainio:master Sep 2, 2025
1 check passed
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.

2 participants