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

ncm-metaconfig: add chrony service #1473

Merged
merged 1 commit into from
Dec 17, 2020
Merged

ncm-metaconfig: add chrony service #1473

merged 1 commit into from
Dec 17, 2020

Conversation

kwaegema
Copy link
Member

chrony service for chrony.conf

ned21
ned21 reviewed Oct 27, 2020
View reviewed changes
Copy link
Contributor

@ned21 ned21 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, aside from a couple of comments about default values and some spacing issues.

Anyone know why the unit test is failing? Looks unrelated.

ncm-metaconfig/src/main/metaconfig/chrony/pan/schema.pan Outdated Show resolved Hide resolved
ncm-metaconfig/src/main/metaconfig/chrony/pan/schema.pan Outdated Show resolved Hide resolved
ncm-metaconfig/src/main/metaconfig/chrony/pan/schema.pan Outdated Show resolved Hide resolved
ncm-metaconfig/src/main/metaconfig/chrony/pan/schema.pan Outdated Show resolved Hide resolved
@ned21 ned21 added this to the 20.12 milestone Oct 27, 2020
@ned21
Copy link
Contributor

ned21 commented Nov 13, 2020

Do we need the merge commit in here? Could you just squash all three commits into one please?

@jrha
Copy link
Member

jrha commented Dec 17, 2020

Rebased and squashed.

jrha
jrha approved these changes Dec 17, 2020
View reviewed changes
Copy link
Member

@jrha jrha left a comment

Choose a reason for hiding this comment

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

LGTM

Like most other PRs against this repository, the Docker tests are failing with:

maven-clean-plugin:2.4.1 or one of its dependencies could not be resolved
The actual component tests are passing without issue.

@jrha jrha merged commit 3d72516 into quattor:master Dec 17, 2020
@ned21
Copy link
Contributor

ned21 commented Dec 18, 2020 

020-12-17T10:56:31.4305136Z [ERROR] Plugin org.apache.maven.plugins:maven-clean-plugin:2.4.1 or one of its dependencies could not be resolved: Failed to read artifact descriptor for org.apache.maven.plugins:maven-clean-plugin:jar:2.4.1: Could not transfer artifact org.apache.maven.plugins:maven-clean-plugin:pom:2.4.1 from/to central (https://repo.maven.apache.org/maven2): Connection timed out (Read failed) -> [Help 1]
2020-12-17T10:56:31.4308840Z [ERROR]

That's not good. Do we need to run clean under Docker since the environment is built afresh each time. I can't remember where the source for /usr/bin/mvn_test.sh is kept. :(

@jrha
Copy link
Member

jrha commented Dec 18, 2020 •
edited
Loading

It's very odd, I can't reproduce it if I build the Dockerfile on my machine, it also successfully fetches a never version of maven-clean-plugin earlier on in the build.

I guess we probably don't need to run clean…

mvn_test.sh currently comes from perl-Test-Quattor but there is a PR open to add it to maven-tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ned21 ned21 ned21 left review comments

@stdweird stdweird stdweird left review comments

@jrha jrha jrha approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
20.12
Development

Successfully merging this pull request may close these issues.
4 participants
@kwaegema @ned21 @jrha @stdweird