-
Notifications
You must be signed in to change notification settings - Fork 230
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
Feat(eos_cli_config_gen): Add support for logging format rfc5424 #3386
Conversation
I found the link to molecule in the PR checkbox comment. Looking through that I couldn't find a test for |
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/logging.j2
Outdated
Show resolved
Hide resolved
Moving to draft until we have tests in here. You can mark is as "ready for review" when it is ready. For Molecule, you should add the new key to cd ansible_collections/arista/avd
molecule converge -s eos_cli_config_gen -- --limit logging |
Should I commit the changes? There are a lot of deletes because of |
It should not delete anything. It sometimes happens if it is the first time you run it. So try to run without limit and then commit the updates. |
Thanks, ran the command again without Edit: ah, sorry, I just noticed I committed the template change with the test artefacts. I hope that's ok? |
We don't care about your commits in your branch. We will squash them into a single commit using the PR title, once we merge it to the devel branch. |
Well, tests passed and the workflow test passed locally earlier once I corrected the capitalization in the schema. Thank you so much for your help! I couldn't have managed the last mile without your help and guidance! |
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.
LGTM. Thank you for this and congratulations on your first contribution to AVD! Looking forward to many more :)
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/logging.j2
Outdated
Show resolved
Hide resolved
…tes/documentation/logging.j2 Co-authored-by: Tony Reddy Goda <[email protected]>
ansible_collections/arista/avd/molecule/eos_cli_config_gen/documentation/devices/logging.md
Outdated
Show resolved
Hide resolved
…umentation/devices/logging.md
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.
Applied review comments from @tgodaA - LGTM
Change Summary
This PR adds support for
logging format rfc5424
Related Issue(s)
Fixes #3365
Component(s) name
arista.avd.eos_cli_config_gen
Proposed changes
This adds support for
logging format rfc5424
.It adds the templates, documentation and schemas for this.
How to test
I'm not sure how to run tests. I'm using vscode and the dev-container included in the repo.
I'm familiar with Makefiles from *BSD, but not while developing.
See separate comment below.
Checklist
User Checklist
Repository Checklist