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

[i304] Rename media-box to post #36

Merged
merged 3 commits into from
Jul 10, 2020
Merged

Conversation

geoff-wb
Copy link
Contributor

@geoff-wb geoff-wb commented Jul 9, 2020

Summary

  • Updating styles as part of the rename from media-box to post
  • A couple other classes also updated based on feedback

Submitter Checklist:

  • Perform self-review (see reviewer checklist)
  • Annotate MR with comments for reviewer
  • Document any new patterns or features on peak-design-system
  • Assign a team member who should specifically review this code
  • Address reviewer feedback, if any, and assign back to reviewer

Reviewer Checklist:

  • Version bump in package.json
  • Visually review significant UI changes
  • Assign @pez-wb as reviewer if needed
  • Assign back to submitter with feedback

@geoff-wb geoff-wb changed the title Rename media-box to post [i304] Rename media-box to post Jul 9, 2020
@@ -1,6 +1,6 @@
{
"name": "@wealthbar/peak-style",
"version": "1.7.6",
"version": "1.7.7",
Copy link
Contributor Author

@geoff-wb geoff-wb Jul 9, 2020

Choose a reason for hiding this comment

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

Not sure if this was necessary. I also bumped the version number in the peak docs

Copy link
Contributor

Choose a reason for hiding this comment

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

pretty sure it is necessary, because it's an NPM package and I don't think NPM wants you to publish the same version twice.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is why the build errors are happening. you best bet is to keep this at the latest version until the peak-style branch w/ your updates have been merged and deployed.

Until then just make a note in the PR description that it relies on a yarn link to your peak-style branch (include PR link).

Copy link
Contributor

@pez-work pez-work Jul 9, 2020

Choose a reason for hiding this comment

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

wrong PR for my comment — should apply to the PDS "@wealthbar/peak-style" dependency

Copy link
Contributor

@pez-work pez-work left a comment

Choose a reason for hiding this comment

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

some cleanup suggestions

scss/patterns/post.scss Outdated Show resolved Hide resolved
@@ -1,6 +1,6 @@
{
"name": "@wealthbar/peak-style",
"version": "1.7.6",
"version": "1.7.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

this is why the build errors are happening. you best bet is to keep this at the latest version until the peak-style branch w/ your updates have been merged and deployed.

Until then just make a note in the PR description that it relies on a yarn link to your peak-style branch (include PR link).

scss/patterns/post.scss Show resolved Hide resolved
@geoff-wb geoff-wb merged commit ea2536d into master Jul 10, 2020
@geoff-wb geoff-wb deleted the i304.change_name_to_post branch July 10, 2020 20:11
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.

3 participants