Skip to content

Conversation

jkronegg
Copy link
Contributor

@jkronegg jkronegg commented Oct 8, 2025

🤔 What's changed?

Minor changes to avoid unnecessary stuff:

  • avoid passing a default null value in AstNode.getSingle
  • avoid duplicate condition checks
  • avoid checking null value on elements that are never null (AstNode.getToken)
  • avoid never used FeatureHeader and RuleHeader default value
  • corrected bad lib scope for assertj-core (was compile-time)

⚡️ What's your motivation?

I didn't check the performance as theses are very small changes. But removing unnecessary code is never a bad thing...

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)

♻️ Anything particular you want feedback on?

I couldn't run make mostlyclean acceptance (these are many errors regarding newlines on Windows).

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

The AssertJ fix is good. I'll pull that out separately.

Beyond that I'm not sure how I feel about this change. Checking pre- and post-conditions catches mistakes in understanding early. The checks are also quite inexpensive. I don't think these milligrams of performance gain are worth the potential headache in the future..

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