-
Notifications
You must be signed in to change notification settings - Fork 275
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
chore(tools): Use PR number in changeset titles when Issue is absent #3152
Conversation
…g changeset with no Issue Previously, it was just leaving it off, but it left things a bit unharmonic/off-balance in terms of the resulting changeset template. I'd also just been manually adding those during the release process thus far, so figured I'd just automate it now. Also, added insta tests that make sure things are rendering properly.
@abernix, please consider creating a changeset entry in |
CI performance tests
|
{{- if issues -}} | ||
{{- if issues }} {{ endif -}} | ||
{{- for issue in issues -}} | ||
([Issue #{issue.number}]({issue.url})) | ||
{{- if not @last }}, {{ endif -}} | ||
{{- endfor -}} | ||
{{ else -}} | ||
{{- if pulls -}} | ||
{{- if pulls }} {{ endif -}} | ||
{{- for pull in pulls -}} | ||
([PR #{pull.number}]({pull.url})) | ||
{{- if not @last }}, {{ endif -}} | ||
{{- endfor -}} | ||
{{- else -}} | ||
{{- endif -}} | ||
{{- endif }} |
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.
Having this more nicely formatted is... nice. :) There are some bug-fixes in there, too, and then the actual feature that this PR claims to add.
By [@{author}](https://github.com/{author}){{ if pulls }} in {{ for pull in pulls -}} | ||
{pull.url}{{ if not @last }}, {{ endif }} | ||
{{- endfor }} | ||
{{- endfor }}{{ endif }} |
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.
Note: There is no condition in the code today that actually results in there being multiple pulls
members, but this template still supports it. A future state can support it easily.
{{- endfor }} | ||
const EXAMPLE_TEMPLATE: &'static str = "### { title } | ||
{{- if issues -}} | ||
{{- if issues }} {{ endif -}} |
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 issues }} {{ endif -}} |
shouldn't this be removed?
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.
Good eye and question, but no and hard to comment in the code (since it's within a template) why it exists. It actually allows me to:
- use a nice formatted structure for this template
- while ensuring that there will be a space preserved there at all times
- while not leaving a trailing whitespace on the
### title
line which would almost certainly get removed by someone's editor.
Previously, it was just leaving it off, but it left things a bit
unharmonic/off-balance in terms of the resulting changeset template. I'd
also just been manually adding those during the release process thus far, so
figured I'd just automate it now.
Also added a lot of tests and fixed a couple bugs with trailing spaces.
Relates to ##2261