Skip to content

Port #49063 to master#58042

Merged
dwoz merged 5 commits intosaltstack:masterfrom
nicholasmhughes:port-49063
Jul 29, 2020
Merged

Port #49063 to master#58042
dwoz merged 5 commits intosaltstack:masterfrom
nicholasmhughes:port-49063

Conversation

@nicholasmhughes
Copy link
Contributor

@nicholasmhughes nicholasmhughes commented Jul 28, 2020 •
edited
Loading

What does this PR do?

Port #49063 to master

What issues does this PR fix or reference?

Fixes: N/A

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@nicholasmhughes nicholasmhughes requested a review from a team as a code owner July 28, 2020 14:48
@ghost ghost requested review from Akm0d and removed request for a team July 28, 2020 14:48
waynew
waynew reviewed Jul 28, 2020
View reviewed changes
waynew
waynew reviewed Jul 28, 2020
View reviewed changes
Akm0d
Akm0d previously approved these changes Jul 28, 2020
View reviewed changes
waynew
waynew suggested changes Jul 28, 2020
View reviewed changes
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

This looks great! I think just adding a changelog entry, perhaps 49063.added with Added support for MSI-style authentication for azurearm. would be reasonable?

Thanks!

@nicholasmhughes
Copy link
Contributor Author

nicholasmhughes commented Jul 28, 2020

@waynew unfortunately, that message would be misleading. the main reason for the original PR has since been merged for a while... I basically just ported over some additional housekeeping that got left behind. after all that, the main thing that's been added was tests... 😆 I don't even know what to call this change...

@waynew
Copy link
Contributor

waynew commented Jul 28, 2020

Well, it does have the else case, so maybe it's just a fixed? 🤷

@nicholasmhughes
Copy link
Contributor Author

nicholasmhughes commented Jul 28, 2020

🤷 took a stab at it

waynew
waynew approved these changes Jul 28, 2020
View reviewed changes
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

Good enough for me! That's at least enough information for someone reading the changelog to be able to say, "Ah, yeah, I need to know more about that". Which is the point of the changelog 😂

🚀

@dwoz dwoz merged commit 4385344 into saltstack:master Jul 29, 2020
@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Akm0d Akm0d Akm0d left review comments

+1 more reviewer

@waynew waynew waynew approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Magnesium Mg release after Na prior to Al master-port

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nicholasmhughes @waynew @Akm0d @dwoz @sagetherage