-
Notifications
You must be signed in to change notification settings - Fork 155
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
Add description
for tests
#2397
Conversation
7a6921d
to
2c66e7c
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.
Looks good to me.
03ba5d8
to
7bf1fec
Compare
7bf1fec
to
4025849
Compare
39f27c8
to
857995f
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 looks good to me. I noticed that sometimes description: >
was used on multi-line descriptions. Should we use that for all multi-line descriptions or not? For non-external tests (tests in coreos-assembler mantle kola) the concept of >
doesn't exist.
The resulting json output from using >
results in a newline character before the description text. Example below:
[
"ext.config.files.check-symlink",
"Verify /etc release symlinks are valid.\nSee\n- https://bugzilla.redhat.com/show_bug.cgi?id=2068148\n- https://github.com/openshift/os/pull/815"
]
[
"ext.config.files.console-config",
"\nVerify that the kargs and grub.cfg commands specified in \nplatforms.json have been properly applied to the image. \nAlso check cosa correctly translated platforms.yaml to \nplatforms.json, by spot-checking certain expected values\nin platforms.json.\n"
]
I don't know if that will ultimately make any difference when we try to import into Polarion.
Agree that it looks ugly for human reading as many |
857995f
to
f6e77c9
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.
This is really great!
SGTM. I will try to drop |
- Remove `description: >` - Replace related links into comments
d664432
to
8fcb398
Compare
Done for the updates, thanks all for the review and suggestions. |
LGTM |
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.
I didn't re-review the full diff in detail, but the interdiff looks good. Nice work @HuijingHei!
Based on coreos/coreos-assembler#3425