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

Respect STACKCTL_DIRECTORY in findRemovedStack #71

Merged
merged 4 commits into from
Apr 8, 2024
Merged

Conversation

pbrisbin
Copy link
Member

@pbrisbin pbrisbin commented Apr 5, 2024

When inferring removed stacks, we use use the fact that a specification
doesn't exist on disk to decide to remove the corresponding stack.
Checking for the specification on disk was not using
STACKCTL_DIRECTORY, so if one were set this would always report the
file as missing and could result in deleting a stack we should not.

@pbrisbin pbrisbin changed the title pb/directory Fix bug in findRemovedStack with non-default STACKCTL_DIRECTORY Apr 5, 2024
@pbrisbin pbrisbin requested a review from chris-martin April 5, 2024 15:24
@pbrisbin

This comment was marked as outdated.

@pbrisbin pbrisbin marked this pull request as ready for review April 5, 2024 15:25
@pbrisbin pbrisbin requested a review from a team as a code owner April 5, 2024 15:25
@pbrisbin pbrisbin requested review from derek-ye and removed request for a team and derek-ye April 5, 2024 15:25
Copy link
Contributor

@chris-martin chris-martin left a comment

Choose a reason for hiding this comment

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

Go ahead with this, I'll minimize what I care about in mine so it doesn't conflict.

@chris-martin

This comment was marked as outdated.

@pbrisbin

This comment was marked as outdated.

@chris-martin

This comment was marked as outdated.

pbrisbin added 3 commits April 8, 2024 15:10
When inferring removed stacks, we use use the fact that a specification
doesn't exist on disk to decide to remove the corresponding stack.
Checking for the specification on disk was not using
`STACKCTL_DIRECTORY`, so if one were set this would always report the
file as missing and could result in deleting a stack we should not.

Ideally, I would have added a failing test before fixing this, but this
area is just not easy to get a test on at this time.
@pbrisbin pbrisbin changed the title Fix bug in findRemovedStack with non-default STACKCTL_DIRECTORY Respect STACKCTL_DIRECTORY in findRemovedStack Apr 8, 2024
@pbrisbin pbrisbin enabled auto-merge (rebase) April 8, 2024 19:16
@pbrisbin pbrisbin merged commit e785b7e into main Apr 8, 2024
5 checks passed
@pbrisbin pbrisbin deleted the pb/directory branch April 8, 2024 19:40
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