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

Recognize OSMC as Debian-based #62199

Merged
merged 1 commit into from
Sep 18, 2022
Merged

Recognize OSMC as Debian-based #62199

merged 1 commit into from
Sep 18, 2022

Conversation

nkuttler
Copy link
Contributor

What does this PR do?

This changes recognizes OSMC as Debian based.

What issues does this PR fix or reference?

Fixes: #62198

Merge requirements satisfied?

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

Commits signed with GPG?

No, sorry

@nkuttler nkuttler requested a review from a team as a code owner June 21, 2022 21:35
@nkuttler nkuttler requested review from dwoz and removed request for a team June 21, 2022 21:35
@nkuttler
Copy link
Contributor Author

nkuttler commented Jun 24, 2022 •
edited
Loading

Should I pull from upstream master and rebase on top? Or add a test somewhere? The change is trivial, please let me know if anything is needed.

@nkuttler nkuttler force-pushed the master branch 2 times, most recently from 6b68e3d to 8f32d81 Compare July 19, 2022 16:50
@nkuttler nkuttler force-pushed the master branch 6 times, most recently from 51fcc0b to f994a8f Compare August 9, 2022 22:00
@nkuttler nkuttler force-pushed the master branch 3 times, most recently from b976978 to b9de78b Compare August 19, 2022 22:00
@nkuttler
Copy link
Contributor Author

@dwoz Is there anything I can do to make the review easier? The failing check seems unrelated.

@nkuttler nkuttler force-pushed the master branch 5 times, most recently from 5cfe9fc to eedea62 Compare August 29, 2022 22:00
@nkuttler nkuttler force-pushed the master branch 3 times, most recently from 9d47f27 to f412369 Compare September 2, 2022 22:00
@garethgreenaway
Copy link
Contributor

@nkuttler Thanks for the PR. If you check salt/tests/pytests/unit/grains/test_core.py and look for the test test_mendel_os_grains, duplicating that test and swapping in the right bits for OSMC should be a sufficient test to get this merged in.

@garethgreenaway
Copy link
Contributor

@nkuttler Looks like a failure in the test. Looking at the new test and the result, it looks like the expected versions should all be 2020 based on the _os_release_map values. Also could you please add a changelog to the changelog directory? Filename should be 62198.fixed based on the issue you're fixing and should contain a short line of what you're fixing. Thanks!

@nkuttler
Copy link
Contributor Author

@garethgreenaway Thanks a lot for looking into this! I think the one check that fails was not related to my latest commit? But the log is a little overwhelming tbh.

@garethgreenaway
Copy link
Contributor

@nkuttler Nope. not related. Looks like a timeout of sorts. I restarted the test.

@garethgreenaway
Copy link
Contributor

All set. Thanks for the contribution!

@garethgreenaway garethgreenaway merged commit 01146e7 into saltstack:master Sep 18, 2022
@nkuttler nkuttler mentioned this pull request Sep 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garethgreenaway garethgreenaway garethgreenaway approved these changes

@dwoz dwoz Awaiting requested review from dwoz dwoz was automatically assigned from saltstack/team-core

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] OSMC is not recognized as Debian-based
2 participants
@nkuttler @garethgreenaway