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.

Sign up for GitHub

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

Jump to bottom

tests : Add tests for CoreSeparator block. #24

Merged
merged 8 commits into from
Sep 26, 2024
Merged

tests : Add tests for CoreSeparator block. #24

merged 8 commits into from
Sep 26, 2024

Conversation

Ta5r
Copy link
Collaborator

@Ta5r Ta5r commented Sep 20, 2024 •
edited by justlevine
Loading

Tracking wpengine#294

What:

This PR backfills tests for the CoreSeparator block and its attributes.

Tested attributes:

  • align
  • anchor
  • backgroundColor
  • className
  • cssClassName
  • gradient
  • lock
  • opacity
  • style

Untested fields:

  • CoreSeparatorAttributes.metadata - @todo

Exposed issues:

NA

@Ta5r Ta5r self-assigned this Sep 20, 2024
justlevine
justlevine requested changes Sep 21, 2024
View reviewed changes
Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

  1. Address the feedback.
  2. Update the PR description to match the format used in tests: Add tests for CoreHeading block #10
  3. Add a changeset (by waiting you're forcing me to review this PR again once you add it)

General Notes (cc @ashutoshgautams )

  • If you think a PR is "ready for review", then mark it Ready for Review. Reserve Draft status for PRs you're actively working on but are looking for interim feedback on.
  • Unless your PR is doing multiple things (ps: it shouldn't), consider squashing your commits into a single one before opening the PR. Commits of you fixing your own unreviewed code are just noise, and clutter the commit history. (Once you iterating based on code review or creating a child branch dependent on the parent one, that's when you stop cleaning up your commit history to avoid merge conflicts).

@justlevine justlevine marked this pull request as ready for review September 21, 2024 18:25
@justlevine justlevine changed the title tests : Add tests for Core Separator block. tests : Add tests for CoreSeparator block. Sep 21, 2024
This was referenced Sep 21, 2024
Merged
Merged
@Ta5r Ta5r marked this pull request as draft September 23, 2024 06:48
justlevine
justlevine requested changes Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@justlevine justlevine left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Test looks good 🙌 still needs a changeset and PR description update before it can be opened upstream.

Mark this ready for review and rerequest a review from me when that's done 🙇

@Ta5r Ta5r marked this pull request as ready for review September 23, 2024 20:24
justlevine
justlevine approved these changes Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

@Ta5r go ahead and open a PR upstream 🙌

@justlevine justlevine mentioned this pull request Sep 23, 2024
Merged
@justlevine justlevine added the has-upstream-pr A PR has been opened against wpengine's repo label Sep 23, 2024
@justlevine justlevine merged commit 05dc20a into main Sep 26, 2024
10 checks passed
@justlevine justlevine deleted the core-separator-test branch October 18, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justlevine justlevine justlevine approved these changes

Assignees

@Ta5r Ta5r

Labels
has-upstream-pr A PR has been opened against wpengine's repo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ta5r @justlevine