-
Notifications
You must be signed in to change notification settings - Fork 40
add missing changelog entries for #631 #652
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
Conversation
PLeVasseur
left a comment
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'm not sure if I'm doing this right 😅
Tried to find if any might be missing.
| - :p:`fls_YDVgFaTQwcL8` | ||
| - :p:`fls_zv73CR8rplIa` | ||
| - :p:`fls_tZJgZDWVChJV` | ||
|
|
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 the "New paragraphs" list for this item, I think we're missing a few IDs from PR #631's spec changes:
| - :p:`fls_LnPDQW3bnNUw` | |
| - :p:`fls_sw6HrsxsnG2y` | |
| - :p:`fls_urIJ5JNHLhm6` | |
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.
skipped those since they are glossary entries, but am not sure I should have... what say @kirtchev-adacore
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.
how I done it in the past is I would define the term inline, not in glossary, and it's then that I would add it to changelog
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 we should treat new paragraphs in the glossary as "regular" new paragraphs, and document them in the changelog.
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 agreed in the FLS meet, I replicated the glossary entry inline so that we can put it in the changelog, since the glossary is informational
also
fls_sw6HrsxsnG2yalready has an entry inlinefls_urIJ5JNHLhm6is a glossary-only reference, pointing to the section where the glossary entry is introduced inline
| - :p:`fls_drb114dtvlpt` | ||
| - :p:`fls_uxysntb3u03j` | ||
| - :p:`fls_vstdqifqipbh` | ||
|
|
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 the "Changed paragraphs" list, two IDs from PR 631's diff look missing. One is a list-structure shift (fls_wh201rmh6u6d), the other switches :dt: to :t: (fls_y3oputy9e0sz).
| - :p:`fls_wh201rmh6u6d` | |
| - :p:`fls_y3oputy9e0sz` | |
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.
skipped those because I see them not as semantic changes
PLeVasseur
left a comment
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.
Thanks @tshepang!
No description provided.