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

stacks: provide stack and config on component creation #36778

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

liamcervante
Copy link
Member

This PR is a refactoring of the Stacks runtime. No expected behaviour change as a result of these changes.

Previously, nodes in the stacks runtime were created with a single link back to the main object. From the main object they could then search for the stack instance the nodes were linked to. However, the nodes are all created from inside the stack they want to link back to. I found this reverse look up very confusing.

After this PR, nodes in the runtime accept both the stack and the configuration they are linked to directly in their constructor. This information is simply always available to them, removing the need for the lookups. I think it is easier to reason about each node and where it lives in the stack with this information made available up front. In addition, we can now create stack instances directly within the embedded stack calls that initialise them, making the tracing of stack instances much easier.

@liamcervante liamcervante requested a review from a team as a code owner March 26, 2025 16:29
@liamcervante liamcervante added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Mar 26, 2025
Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

LGTM!

@liamcervante liamcervante merged commit 7bec38b into main Apr 1, 2025
9 of 10 checks passed
@liamcervante liamcervante deleted the liamcervante/stacks/prevent-lookups branch April 1, 2025 06:34
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.

2 participants