Skip to content
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

USWDS-Site: Add changelog entries for #5358 #2160

Merged
merged 6 commits into from
Jul 28, 2023

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Jun 29, 2023

Summary

Added changelog entries for uswds/uswds#5358

Updated the ellipsis color to meet color contrast requirements.
Updated styles to respect the value added to $theme-pagination-background-color.

Preview link

Pagination changelog

@amyleadem amyleadem changed the title USWDS-Site: Add changelog entry for #5358 USWDS-Site: Add changelog entries for #5358 Jun 29, 2023
@amyleadem amyleadem marked this pull request as ready for review June 30, 2023 16:42
Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on combining these notes into one? They both come from a single code change. Otherwise LGTM.

@amyleadem
Copy link
Contributor Author

amyleadem commented Jul 12, 2023

@mejiaj That's a good question. I could really go either way on combining them. I lean towards keeping them separate for a couple of reasons:

  1. Only one item is a breaking change and I like being able to focus the breaking entry on related user action
  2. The changes feel distinct to me and it feels easier to scan if they are separate entries

@amyleadem amyleadem requested a review from mejiaj July 12, 2023 20:57
Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I'm indecisive about splitting into two or combining into one changelog. Maybe we should establish a pattern for if each changelog item is per change or per PR.

@amyleadem
Copy link
Contributor Author

amyleadem commented Jul 14, 2023

@mahoneycm @mejiaj
Our changelog template README currently recommends breaking up features into multiple data items:

If a single release item accomplished multiple tasks, break the tasks up into separate data items. The goal is to itemize and highlight user benefits and actions required.

I believe this was born out of some of the earlier discussions about changelogs. Happy to discuss this more if you are inclined to change the standard. We also could refine the README to be more clear if it feels muddled.

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amyleadem thanks for the feedback. Approving since there doesn't be a strong reason to combine them.

@amyleadem amyleadem changed the base branch from main to release-3.5.1 July 21, 2023 19:32
Copy link
Member

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I support Amy's reasoning for a per-change approach to changelogs. A single PR may have multiple changelog-worthy items.

Copy link
Member

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the changelogs to add dummy version number

_data/changelogs/component-pagination.yml Outdated Show resolved Hide resolved
_data/changelogs/component-pagination.yml Outdated Show resolved Hide resolved
@thisisdano thisisdano merged commit 364d0d1 into release-3.5.1 Jul 28, 2023
1 check failed
@thisisdano thisisdano deleted the al-changelog-5358 branch July 28, 2023 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants