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

Installing chrony removes systemd timesyncd #79

Merged
merged 1 commit into from
May 1, 2023
Merged

Installing chrony removes systemd timesyncd #79

merged 1 commit into from
May 1, 2023

Conversation

kdebisschop
Copy link
Contributor

Overall Review of Changes:
Removes check of systemd-timesyncd when chrony is the configured time source

Issue Fixes:
#78

Enhancements:
Please list any enhancements/features that are not open issue tickets

How has this been tested?:
Used to configure an Ubuntu 20 test server. The step which previously failed (because systemd-timesyncd cannot be installed if chrony is) now runs successfully.

github-actions[bot]
github-actions bot reviewed Apr 18, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on opening your first pull request and thank you for taking the time to help improve Ansible-Lockdown!
Please join in the conversation happening on the Discord Server as well.

@kdebisschop kdebisschop force-pushed the installing-chrony-removes-systemd-timesyncd branch from 642f1b4 to 4bd47f7 Compare April 18, 2023 19:54
@MrSteve81
Copy link
Contributor

MrSteve81 commented Apr 20, 2023 •
edited
Loading

Please make sure you sign off on your commits. The DCO is failing because of that.

@kdebisschop
Copy link
Contributor Author

I saw that the note yesterday but have not yet had time to fix and do a force push. I hope to do that tonight. Thanks for the reminder.

@kdebisschop kdebisschop force-pushed the installing-chrony-removes-systemd-timesyncd branch 2 times, most recently from df00c15 to 4e24c68 Compare April 22, 2023 01:57
georgenalen
georgenalen approved these changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@georgenalen georgenalen left a comment

Choose a reason for hiding this comment

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

Looks good!

MrSteve81
MrSteve81 reviewed Apr 27, 2023
View reviewed changes
Copy link
Contributor

@MrSteve81 MrSteve81 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Added to my local machine and tested looks good.

@MrSteve81 MrSteve81 self-requested a review April 27, 2023 14:36
MrSteve81
MrSteve81 previously approved these changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@MrSteve81 MrSteve81 left a comment

Choose a reason for hiding this comment

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

Added to my local machine and tested looks good.

georgenalen
georgenalen requested changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@georgenalen georgenalen left a comment

Choose a reason for hiding this comment

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

Commit is good, we just noticed their is no GPG signature.

@georgenalen
Copy link
Contributor

@kdebisschop,
He we just noticed that your commit has the signed-off-by, but does not have a gpg signature. For some reason DCO didn't pick up that missing element. Can you go back into your commit and make sure that commit is gpg signed? Your commit should have the green Verified next to it if it's gpg signed.

-George

@kdebisschop
only stop and mask systemd-timesyncd if package is installed
919d71a 
Signed-off-by: Karl DeBisschop <kdebisschop@gmail.com>
@kdebisschop kdebisschop force-pushed the installing-chrony-removes-systemd-timesyncd branch from 4e24c68 to 919d71a Compare April 27, 2023 21:45
@kdebisschop
Copy link
Contributor Author

I have just force pushed the commit after running git commit -S --amend

I hope that is what you want

@MrSteve81 MrSteve81 requested a review from georgenalen April 28, 2023 11:37
georgenalen
georgenalen approved these changes May 1, 2023
View reviewed changes
Copy link
Contributor

@georgenalen georgenalen left a comment

Choose a reason for hiding this comment

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

looks good!

@MrSteve81 MrSteve81 self-requested a review May 1, 2023 15:02
MrSteve81
MrSteve81 approved these changes May 1, 2023
View reviewed changes
Copy link
Contributor

@MrSteve81 MrSteve81 left a comment

Choose a reason for hiding this comment

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

Looks Good Thanks For Updating

@MrSteve81 MrSteve81 merged commit 767cdc8 into ansible-lockdown:devel May 1, 2023
@uk-bolly uk-bolly mentioned this pull request Jul 19, 2023
Merged
@kdebisschop kdebisschop deleted the installing-chrony-removes-systemd-timesyncd branch January 28, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@georgenalen georgenalen georgenalen approved these changes

@MrSteve81 MrSteve81 MrSteve81 approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@kdebisschop @MrSteve81 @georgenalen