-
Notifications
You must be signed in to change notification settings - Fork 247
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
TESTS: Ported over dyndns from downstream to upstream #6344
Conversation
4d31588
to
1aad0bf
Compare
1aad0bf
to
8d97426
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the code looks good. I have mostly a small suggestions to imporove it,
8d97426
to
9e8b648
Compare
I updated the lines, and noticed the wrap is set to 120 instead of 100, it should be easier to read now. |
9e8b648
to
af3e2a1
Compare
The decorator @staticmethod needs to be added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor things to take a look at.
af3e2a1
to
1056626
Compare
1056626
to
91599f5
Compare
91599f5
to
4d80d8c
Compare
* Updated code to use current libs from factory.py * Created fixtures to add an additional zone file, additional interface, retrieve the PTR record * Added bind-utils as a required package to utils.py Signed-off-by: Dan Lavu <dlavu@redhat.com>
4d80d8c
to
b14b78a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, LTGM. we can cover some of the other non-vital suggestions in the next PR which adds more test cases.
There are a lot of changes on the next patch. #6363 |
@justin-stephenson can you tell why the flake8 tests here are failing now? |
It looks like the GH Action failed when attempting to post the results into the PR, but running flake8 manually shows:
|
@sidecontrol Is any comment from @jakub-vavra-cz pending to merge this PR? |
There is another PR that is a branch of, this will not pass any of tests so I'm creating a new PR so I can drop this intermediary branch. |
Signed-off-by: Dan Lavu dlavu@redhat.com