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

win_lgpo - additional delvals fixes and string value fix #56569

Merged
merged 8 commits into from
Oct 7, 2020

Conversation

lomeroe
Copy link
Contributor

@lomeroe lomeroe commented Apr 7, 2020

What does this PR do?

Ports fixes from #56060 to other places where the delvals regex was searched.

Corrects an issue where a string value with a semicolon in it would only show the first value in the setting

Adds testing of policy returned from lgpo.get for each ADMX policy test

What issues does this PR fix or reference?

#56062

Previous Behavior

Some delvals items would show "**delvals" as the configuration instead of "Disabled"
Items with a semicolon in them (such as the "Enter fully qualified server names separated by semicolons" of "Point and Print Restrictions") would only show the entry up to the semicolon - subsequent runs of lgpo may then reset the value to the shortened string.

New Behavior

Delval items correctly display the policy setting in lgpo.get
Items with a semicolon show all entries

Tests written?

Yes

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

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

@lomeroe lomeroe requested a review from a team as a code owner April 7, 2020 17:39
@ghost ghost requested a review from Ch3LL April 7, 2020 17:39
@Ch3LL Ch3LL self-assigned this Apr 9, 2020
@Ch3LL Ch3LL removed the request for review from a team April 15, 2020 14:13
twangboy
twangboy previously approved these changes Apr 17, 2020
@twangboy
Copy link
Contributor

@lomeroe Need to address the test failures (lint, docs, and pre-commit). And it probably needs a rebase...

Ch3LL
Ch3LL previously requested changes Aug 3, 2020
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

any chance you can follow up on @twangboy 's comments here? #56569 (comment)

@twangboy
Copy link
Contributor

twangboy commented Oct 5, 2020

@lomeroe I rebased and resolved some conflicts here... would you mind making sure I didn't mess this up?

@twangboy twangboy added Magnesium Mg release after Na prior to Al and removed has-failing-test labels Oct 6, 2020
@twangboy twangboy dismissed Ch3LL’s stale review October 6, 2020 14:57

It has been addressed

@lomeroe
Copy link
Contributor Author

lomeroe commented Oct 7, 2020

@twangboy still looks okay to me, thanks for cleaning this up

@dwoz dwoz merged commit f6f5395 into saltstack:master Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Magnesium Mg release after Na prior to Al
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants