-
Notifications
You must be signed in to change notification settings - Fork 74
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
Use YAML literal block syntax for description, rationale #1140
Conversation
This prevents some syntax issues with YAML, e.g. '#' can be used in a literal block without being treated as a line comment. The switch from PyYAML to ruamel.yaml was needed to validate the syntax style.
if not isinstance(item['bug'], str) or not item['bug'].startswith("https://bugzilla.mozilla.org/show_bug.cgi?id="): | ||
self.log_error(f"'bug' must be a URL starting with 'https://bugzilla.mozilla.org/show_bug.cgi?id=', got {item['bug']}", key, line) | ||
self.log_error(f"'bug' must be a URL starting with 'https://bugzilla.mozilla.org/show_bug.cgi?id=', got {item['bug']}.", key) |
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.
If we're requiring this, why not include just the bug number (and take the https://bugzilla...
as implied?)
Not for this change, but as a followup perhaps.
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 think it's easier for contributors to paste the full URL for all of the URLs than to find out what substring should be used. Though new issues will have the URLs in the issue OP instead, so it doesn't matter so much.
if 'caniuse' in item and item['caniuse'] not in [None, '']: | ||
if not isinstance(item['caniuse'], str) or not item['caniuse'].startswith("https://caniuse.com/"): | ||
self.log_error(f"'caniuse' must be a URL starting with 'https://caniuse.com/' or empty, got {item['caniuse']}", key, line) | ||
self.log_error(f"'caniuse' must be a URL starting with 'https://caniuse.com/', got {item['caniuse']}.", key) |
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.
As above.
|
||
if 'mdn' in item and item['mdn'] not in [None, '']: | ||
if not isinstance(item['mdn'], str) or not item['mdn'].startswith("https://developer.mozilla.org/en-US/"): | ||
self.log_error(f"'mdn' must be a URL starting with 'https://developer.mozilla.org/en-US/', got {item['mdn']}.", key) |
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.
As above
data[key][field] = LiteralScalarString(value) | ||
else: | ||
self.log_error(f"'{field}' must use literal block syntax (|).", key) | ||
elif not value.endswith('\n'): |
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.
Is it even possible to have a value that doesn't have a trailing \n
?
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.
Yes:
foo: |-
bar
This prevents some syntax issues with YAML, e.g. '#' can be used in a literal block without being treated as a line comment.
The switch from PyYAML to ruamel.yaml was needed to validate the syntax style.