Skip to content
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

matrix: don't overwrite matrix eval context on attr only matrices #141

Merged
merged 2 commits into from
May 29, 2024

Conversation

ryancragun
Copy link
Collaborator

Matrix blocks are used in several places and depending on the context we we need slightly different behavior when decoding them.

When we're decoding a "full matrix", i.e. one that must have variants and may have include and exclude sub-blocks, we want to first decode the attributes and update the matrix eval context so that it's available when decoding the next attribute or the the later blocks.

When we're decoding include or exclude blocks, they themselves are also decoded as an attrubute only matrix, but we don't want to update the matrix eval context and instead want to inherit the parent matrix's eval context.

This change makes that so, fixing a bug whereby the matrix block only included the parents eval context on the first include or exclude block.

We also end up pinning the version of random in our acceptance tests because a newly released version broke the test output that we expect. Since we only care about getting a diagnostic warning it should be fine to pin to that version.

How to read this pull request

You may provide an optional explanation of the best way a review might
approach the changes that are being proposed in this pull request.

Checklist

  • The commit message includes an explanation of the changes
  • Manual validation of the changes have been performed (if possible)
  • New or modified code has requisite test coverage (if possible)
  • I have performed a self-review of the changes
  • I have made necessary changes and/or pull requests for documentation
  • I have written useful comments in the code

Matrix blocks are used in several places and depending on the context we
we need slightly different behavior when decoding them.

When we're decoding a "full matrix", i.e. one that must have variants and
may have `include` and `exclude` sub-blocks, we want to first decode the
attributes and update the `matrix` eval context so that it's available
when decoding the next attribute or the the later blocks.

When we're decoding `include` or exclude `blocks`, they themselves are
also decoded as an attrubute only matrix, but we don't want to update
the `matrix` eval context and instead want to inherit the parent
matrix's eval context.

This change makes that so, fixing a bug whereby the `matrix` block only
included the parents eval context on the first `include` or `exclude`
block.

We also end up pinning the version of `random` in our acceptance tests
because a newly released version broke the test output that we expect.
Since we only care about getting a diagnostic warning it should be fine
to pin to that version.

Signed-off-by: Ryan Cragun <[email protected]>
@ryancragun ryancragun added the changelog/bug Fix for something that wasn't working. Will be included in "Bug Fixes" category in release notes. label May 29, 2024
@ryancragun ryancragun requested a review from a team as a code owner May 29, 2024 17:42
@jasonodonnell jasonodonnell self-requested a review May 29, 2024 17:50
@ryancragun ryancragun merged commit 5e72174 into main May 29, 2024
5 checks passed
@ryancragun ryancragun deleted the ryan/matrix-context-all-blocks branch May 29, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/bug Fix for something that wasn't working. Will be included in "Bug Fixes" category in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants