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

[3006.x] fixes #65340 correct use of egrep to parse semanage output #65341

Closed
wants to merge 1 commit into from

Conversation

ndptech
Copy link

@ndptech ndptech commented Oct 4, 2023

What does this PR do?

What issues does this PR fix or reference?

Fixes: #65340

Commits signed with GPG?

Yes

@ndptech ndptech requested a review from a team as a code owner October 4, 2023 09:42
@ndptech ndptech requested review from garethgreenaway and removed request for a team October 4, 2023 09:42
@welcome
Copy link

welcome bot commented Oct 4, 2023

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title fixes #65340 correct use of egrep to parse semanage output [3006.x] fixes #65340 correct use of egrep to parse semanage output Oct 4, 2023
@anilsil anilsil added this to the Sulfur v3006.5 milestone Oct 4, 2023
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.

This will require test coverage and a changelog

@dmurphy18
Copy link
Contributor

dmurphy18 commented Nov 8, 2023

@ndptech FYI for any PR to be accepted, it requires tests (written using pytest) and a changelog, these were requested a month ago. Are you still interested in continuing this PR ?

A rebase against the latest 3006.x branch would not be amiss too

@ndptech
Copy link
Author

ndptech commented Nov 8, 2023

@dmurphy18 I am still interested in continuing, but have been very busy.
I'm not familiar with your test suite, and there didn't appear to be any existing tests that were in the same area to use as a base for building new tests - so it may take a while for me to work out what is really needed.
Ultimately there is a fault in 3006.3 and 3006.4 which this patch resolves.

@dmurphy18
Copy link
Contributor

@ndptech Do you mind if I take your PR and run with it, making the changelog and tests for it, and have you review the changes, along with the core team.

@ndptech
Copy link
Author

ndptech commented Nov 8, 2023

@dmurphy18 That's absolutely fine.

@dmurphy18
Copy link
Contributor

dmurphy18 commented Nov 9, 2023

@ndptech This PR is superseded by PR #65524, please review that PR at your leisure and consider closing this PR.
And as I was clearing up my browser tabs, only then noticed the ' quotes, previously only saw the space 🤦 . I actually do have new glasses on order from Zenni Optical 😆
Sorry about that, but please take a look at the changelog and tests written in the other PR, should give idea on how to do it if you have other contributions in the future.

@ndptech ndptech closed this Nov 9, 2023
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.

4 participants