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

fix(PPDSC-2611): fix pipeline #596

Merged
merged 4 commits into from
Feb 7, 2023
Merged

fix(PPDSC-2611): fix pipeline #596

merged 4 commits into from
Feb 7, 2023

Conversation

mstuartf
Copy link
Contributor

@mstuartf mstuartf commented Jan 31, 2023 •
edited
Loading

PPDSC-2611

What

  1. Background - why this is needed
  • Error in main job script
  1. What did you do
  • Prevent logged values being returned by check updates script
  1. What does the reviewers should expect
  • Baselines should update correctly

Updates required case:
Screenshot 2023-01-31 at 11 42 30

No updates required case:
Screenshot 2023-01-31 at 11 50 33

Branch not found case:
Screenshot 2023-01-31 at 15 58 24

Token not found case:
Screenshot 2023-01-31 at 15 58 10

I have done:

  • Written unit tests against changes
  • Written functional tests against the component and/or NewsKit site
  • Updated relevant documentation

I have tested manually:

  • The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
  • The screen reader reads and flows through the elements as expected.
  • There are no new errors in the browser console coming from this PR.
  • When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
  • The Playground feature is working as expected

Before:

After:

Who should review this PR:

How to test:

@mstuartf mstuartf added the draft This is a draft PR and not intended for formal review yet label Jan 31, 2023
@mstuartf mstuartf requested a review from a team as a code owner January 31, 2023 11:33
@github-actions github-actions bot added the fix This change fixes a bug label Jan 31, 2023
@pp-serviceaccount
Copy link
Collaborator

You can preview these changes on:
Documentation Site: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2611-fix
Storybook: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2611-fix/storybook

@mstuartf mstuartf added ready for review Please assist in getting this reviewed draft This is a draft PR and not intended for formal review yet and removed draft This is a draft PR and not intended for formal review yet ready for review Please assist in getting this reviewed labels Jan 31, 2023
@mstuartf mstuartf added ready for review Please assist in getting this reviewed and removed draft This is a draft PR and not intended for formal review yet labels Jan 31, 2023
Xin00163
Xin00163 reviewed Jan 31, 2023
View reviewed changes
scripts/check-baseline-updates.js Outdated
@@ -48,7 +55,7 @@ async function getPercyBuildForBranch(branchName) {
throw Error();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we could add the error message in throw? In the last test, this log is not printed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good suggestion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. It will now always log the error and the typo in the loop (< instead of <= is fixed).

@mstuartf mstuartf merged commit 1c001a9 into main Feb 7, 2023
@mstuartf mstuartf deleted the fix/PPDSC-2611-fix branch February 7, 2023 17:49
mstuartf added a commit that referenced this pull request Feb 20, 2023
@mstuartf
fix(PPDSC-2611): Revert "fix(PPDSC-2611): fix pipeline (#596)"
6a9384e 
This reverts commit 1c001a9.
mstuartf added a commit that referenced this pull request Feb 21, 2023
@mstuartf
Fix/ppdsc 2611 revert (#656)
9bdd5ab 
* fix(PPDSC-2611): Revert "Fix/ppdsc 2611 fix 1 (#629)"

This reverts commit 7ee4a68.

* fix(PPDSC-2611): Revert "fix(PPDSC-2611): fix pipeline (#596)"

This reverts commit 1c001a9.

* fix(PPDSC-2611): Revert "chore(PPDSC-2611): fewer tests for baseline updates (#588)"

This reverts commit fd4d458.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnTParsons JohnTParsons JohnTParsons approved these changes

@Xin00163 Xin00163 Xin00163 approved these changes

Assignees
No one assigned
Labels
fix This change fixes a bug ready for review Please assist in getting this reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mstuartf @pp-serviceaccount @Xin00163 @JohnTParsons