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

Force [email protected] to temporarily work around bug in library #495

Merged
merged 5 commits into from
Apr 5, 2022
Merged

Force [email protected] to temporarily work around bug in library #495

merged 5 commits into from
Apr 5, 2022

Conversation

jameswburke
Copy link
Contributor

@jameswburke jameswburke commented Apr 5, 2022 •
edited
Loading

Summary

This PR, when applied, will force [email protected] to temporarily work around a bug in the library.

This addresses issue #494.

Testing Locally

See How to test changes to Larva packages locally using npm link in order to fully replicate this locally, along with the instructions in #494.

Also, see this thread in Slack for confirmation by Rich Salvucci.

Tickets

https://jira.pmcdev.io/browse/GUT-289

Make sure you complete these items:

  • Updated root CHANGELOG.md with summary of changes under Unpublished section
  • npm run prod in this repo outputs expected changes (excepting the issue with re-ordered partials in larva-css algorithms partials - see LRVA-1885)
  • [o] If adding a new pattern, in the PR comment, included a screenshot and link to the static Vercel deployment
  • [o] If changes to build scripts or the Node.js server, tested changes in pmc-spark via a pre-release
    • If changes to build tools: npm scripts prod, lint, and dev scripts run as expected
    • If changes to Larva server: Static site generates as expected in a theme (avail. on a URL {brand}.stg.larva.pmcdev.io)

@vercel
Copy link

vercel bot commented Apr 5, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/penske-media-corp/pmc-larva/2sgAZgqXuUYrTYFUjwmohXgDHWYJ
✅ Preview: https://pmc-larva-git-bug-gut-289twing-version-penske-media-corp.vercel.app

rsalvucci
rsalvucci approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@rsalvucci rsalvucci left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@laras126
Copy link
Contributor

laras126 commented Apr 5, 2022

Looks like the backstop tests are failing due to a pixel shift issue :-/

laras126
laras126 reviewed Apr 5, 2022
View reviewed changes
package.json
@@ -26,7 +26,12 @@
"sync-tokens": "mkdir -p build/tokens && cp -r packages/larva-tokens/build/ build/tokens/",
"sync-public": "mkdir -p packages/larva/public && cp -r public packages/larva",
"sync-built-js": "mkdir -p build/js/standalone && rm -r build/js/standalone && cp -r packages/larva-js/build build/js/standalone",
"sync-build": "npm run sync-public && npm run sync-tokens && npm run sync-built-js && mkdir -p packages/larva/build && rm -r packages/larva/build && cp -r build/ packages/larva/build"
"sync-build": "npm run sync-public && npm run sync-tokens && npm run sync-built-js && mkdir -p packages/larva/build && rm -r packages/larva/build && cp -r build/ packages/larva/build",
"link-all": "npm run prod && npm run link-larva && npm run link-patterns && npm run link-css && npm run link-js",
Copy link
Contributor

Choose a reason for hiding this comment

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

<3 Can you update confluence to include these commands as well?

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, https://confluence.pmcdev.io/display/ENG/How+to+test+changes+to+Larva+packages+locally+using+npm+link

@laras126
Copy link
Contributor

laras126 commented Apr 5, 2022

Looks like the backstop tests are failing due to a pixel shift issue :-/

To see the issue, go to the build summary and download the failed build's backstop-results artifact - https://github.com/penske-media-corp/pmc-larva/actions/runs/2096756237
Then view html_report/index.html in the browser.

@jameswburke jameswburke merged commit 29581ed into master Apr 5, 2022
@jameswburke jameswburke deleted the bug/GUT-289/twing-version branch April 5, 2022 17:06
@jameswburke jameswburke mentioned this pull request Apr 5, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laras126 laras126 laras126 left review comments

@rsalvucci rsalvucci rsalvucci approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jameswburke @laras126 @rsalvucci