Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Nit: did you want to add PR number(s) here?
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.
Please don’t do this indiscriminately.
Issue numbers are useful in release notes because they help affected users understand that a fix is for them. Unaffected users might go there to understand the context of the change more deeply.
PR numbers are just showing how the sausage is made.
A few cases where it might be appropriate to include PR numbers:
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.
Makes sense to me, thanks @wilhuff for the thoughtful note.
Do you think there's a difference of including them here vs including them in the release notes on the Firebase site? I can see that's a strong case for not including them on the Firebase site but including them here could be helpful for those more curious since they're already in the repo itself.
I don't have super strong feelings about including them here, but wasn't sure if they'd cause much harm.
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.
In this case, there was a series of PR's, so it would be difficult to associate even if we wanted to.
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.
Anyone sufficiently curious can
git log master
so our goal should be to help less interested parties establish context on changes without unnecessarily inflicting the development process on them.