Skip to content

Conversation

@liamcervante
Copy link
Member

This PR updates removed blocks in stacks so that you can target components within nested stacks. This means that users can change input values into embedded stacks that might edit the number of components internally, and add removed blocks within the root stack that target any components removed by the change.

The general approach is that the from attribute of removed blocks now resolves to a full absolute address. Stack instances are now initialised with any external removed blocks in the constructor. So parent stacks pass in any removed blocks they have that target child stacks directly into the child stack. Then the removed blocks are still processed by the stack that actually contains the components that they target, this allows us to ensure that component and removed blocks don't target the same instance easily, and that removed blocks defined locally don't clash with removed blocks defined externally.

@liamcervante liamcervante added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Mar 27, 2025
@liamcervante liamcervante requested a review from a team as a code owner March 27, 2025 09:55
Base automatically changed from liamcervante/stacks/removed-component-refactor to main April 1, 2025 07:08
@liamcervante liamcervante force-pushed the liamcervante/stacks/removed-chain-1/remove-embedded-components branch from ffa57f4 to 4881ea2 Compare April 1, 2025 07:12
cProto := &stacks.FindStackConfigurationComponents_Removed_Block{
SourceAddr: rc.FinalSourceAddr.String(),
ComponentAddr: stackaddrs.Config(stackAddr, stackaddrs.Component{Name: rc.FromComponent.Name}).String(),
ComponentAddr: stackaddrs.Config(stackAddr, stackaddrs.Component{Name: rc.From.ConfigComponent().String()}).String(),
Copy link
Member Author

Choose a reason for hiding this comment

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

not correct! we need to combine the addresses properly

stackaddrs.ConfigComponent{
Stack: append(stackAddr, rc.From.ConfigComponent().Stack)
Component: rc.From.ConfigComponent().Component
}

Copy link
Member

Choose a reason for hiding this comment

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

Or simply c.From.ConfigComponent().String()? The definition already contains the stack string

func (ist InStackConfig[T]) String() string {
...
return ist.Stack.String() + "." + ist.Item.String()

Copy link
Member Author

Choose a reason for hiding this comment

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

we do need to combine it with the other stack address as the from address is relative, and we want to provide the absolute address here.

@liamcervante liamcervante force-pushed the liamcervante/stacks/removed-chain-1/remove-embedded-components branch from 47fdf49 to 3780e70 Compare April 2, 2025 13:24
@liamcervante liamcervante force-pushed the liamcervante/stacks/removed-chain-1/remove-embedded-components branch from 3780e70 to dd7b3ba Compare April 2, 2025 13:25
@liamcervante liamcervante merged commit fbd5a79 into main Apr 3, 2025
8 checks passed
@liamcervante liamcervante deleted the liamcervante/stacks/removed-chain-1/remove-embedded-components branch April 3, 2025 08:29
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2025

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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