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.

Sign up for GitHub

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

Jump to bottom

Explicity pass context for salt-ssh state calls #63184

Merged
merged 2 commits into from Dec 8, 2022
Merged

Explicity pass context for salt-ssh state calls #63184

merged 2 commits into from Dec 8, 2022

Conversation

Ch3LL
Copy link
Contributor

@Ch3LL Ch3LL commented Dec 2, 2022 •
edited

Fixes: #60003

@Ch3LL Ch3LL requested a review from a team as a code owner December 2, 2022 15:14
@Ch3LL Ch3LL requested review from cmcmarrow and removed request for a team December 2, 2022 15:14
This was referenced Dec 2, 2022
Closed
Closed
@Ch3LL Ch3LL changed the title [WIP] Explicity pass context for salt-ssh highstate [WIP] Explicity pass context for salt-ssh state calls Dec 2, 2022
@Ch3LL Ch3LL changed the title [WIP] Explicity pass context for salt-ssh state calls Explicity pass context for salt-ssh state calls Dec 2, 2022
@Ch3LL Ch3LL mentioned this pull request Dec 2, 2022
Closed
@jpmckinney jpmckinney mentioned this pull request Dec 3, 2022
Closed
3 tasks
Rudd-O
Rudd-O previously approved these changes Dec 5, 2022
View reviewed changes
Copy link

@Rudd-O Rudd-O left a comment •
edited

Choose a reason for hiding this comment

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

Whoa this looks nice. Honestly, the functions being called should have never gained optional context parameters — that pattern was always a recipe that introduced fragility for maintainers and hid code smells. Hoping that Salt code will gain some type checking soon, to make these problems easier to solve.

Please ensure that your PR complies with the stated requirements for merging — including but not limited to fixing the testing. Thanks.

@Ch3LL Ch3LL added the Sulfur v3006.0 release code name and version label Dec 5, 2022
@Ch3LL Ch3LL force-pushed the ssh_map branch 2 times, most recently from e8148dc to 13afb21 Compare December 6, 2022 16:12
cmcmarrow
cmcmarrow previously approved these changes Dec 6, 2022
View reviewed changes
@Ch3LL Ch3LL requested a review from cmcmarrow December 7, 2022 20:17
cmcmarrow
cmcmarrow approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@cmcmarrow cmcmarrow left a comment

Choose a reason for hiding this comment

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

Should probably get the commits signing again.

@Ch3LL Ch3LL merged commit 5b08153 into saltstack:master Dec 8, 2022
@stratusjerry stratusjerry mentioned this pull request Jan 20, 2023
Open
@jpmckinney jpmckinney mentioned this pull request Feb 27, 2023
Merged
3 tasks
@mwarkentin mwarkentin mentioned this pull request Mar 9, 2023
Merged
@twangboy twangboy mentioned this pull request Sep 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmcmarrow cmcmarrow cmcmarrow approved these changes

@Rudd-O Rudd-O Rudd-O left review comments

Assignees
No one assigned
Labels
Sulfur v3006.0 release code name and version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] salt-ssh support for extra_filerefs in Saltfile is broken
3 participants
@Ch3LL @Rudd-O @cmcmarrow