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(build): sw-460 npm updates #958

Merged
merged 1 commit into from
Jul 27, 2022
Merged

fix(build): sw-460 npm updates #958

merged 1 commit into from
Jul 27, 2022

Conversation

cdcabrera
Copy link
Member

@cdcabrera cdcabrera commented Jul 27, 2022 •
edited
Loading

What's included

  • fix(build): sw-460 npm updates

How to test

Coverage and basic unit test check

  1. update the NPM packages with $ yarn
  2. $ yarn test
  3. confirm tests come back clean

Local run check

  1. update the NPM packages with $ yarn
  2. $ yarn start
  3. confirm this fails, this is being handled via PR fix(build): SW-460 dev standalone local run #957

Proxy run check

  1. update the NPM packages with $ yarn
  2. make sure Docker is running, plus on network, then
  3. $ yarn start:proxy
  4. confirm you can log in and view the application and subsequent products

Check the build

  1. update the NPM packages with $ yarn
  2. $ yarn build
  3. confirm the build completes successfully and without error

Example

...

Updates issue/story

sw-460
related to #957
related to #954

@cdcabrera cdcabrera added build 202208 project phase labels Jul 27, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022 •
edited
Loading

Codecov Report

Merging #958 (cb0c681) into dev (dde0693) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #958      +/-   ##
==========================================
+ Coverage   93.92%   93.96%   +0.03%     
==========================================
  Files         129      129              
  Lines        4036     4044       +8     
  Branches     1580     1583       +3     
==========================================
+ Hits         3791     3800       +9     
+ Misses        227      226       -1     
  Partials       18       18
Impacted Files Coverage Δ
src/components/i18n/i18n.js 87.17% <ø> (ø)
src/services/platform/platformServices.js 98.14% <ø> (ø)
src/common/helpers.js 100.00% <100.00%> (+1.66%) ⬆️
src/components/router/redirect.js 100.00% <100.00%> (ø)
src/components/router/routerHelpers.js 98.80% <100.00%> (+0.10%) ⬆️
src/components/toolbar/toolbarContext.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dde0693...cb0c681. Read the comment docs.

@cdcabrera cdcabrera marked this pull request as ready for review July 27, 2022 15:47
cdcabrera
cdcabrera commented Jul 27, 2022
View reviewed changes
src/components/pagination/__tests__/__snapshots__/pagination.test.js.snap
@@ -65,7 +66,7 @@ exports[`Pagination Component should render a non-connected component: non-conne
"items": "",
"itemsPerPage": "Items per page",
"ofWord": "of",
"optionsToggle": "Items per page",
"optionsToggle": "",
Copy link
Member Author

Choose a reason for hiding this comment

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

optional/future:
we may have to set this prop manually now, confusing why PF decided to not continue pulling placeholder like content for this. keep an eye on it

cdcabrera
cdcabrera commented Jul 27, 2022
View reviewed changes
Copy link
Member Author

@cdcabrera cdcabrera 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.

optional/future:
looks like local run with start:proxy highlights an icon misalignment on the graph legend... basically the pretty squares don't align veritcally. We've seen this before with PF updates, and it does appear consistent with what is currently in prod. Sometimes it gets corrected by moving stuff into environment, missing platform stylesheet locally, etc.

keep an eye on it... if it bugs the designers we'll need to try aligning them
Screen Shot 2022-07-27 at 11 56 14 AM

@bclarhk

cdcabrera
cdcabrera commented Jul 27, 2022
View reviewed changes
Copy link
Member Author

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

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

There's also a future local run set of warnings via Victory charts where they attempted to include source maps for local dev, but still managed to include the refs in their NPM dist... we're ignoring the warnings and continuing the update, but heads up...

related

@cdcabrera cdcabrera merged commit 9fec20c into RedHatInsights:dev Jul 27, 2022
@cdcabrera cdcabrera mentioned this pull request Jul 27, 2022
Merged
@cdcabrera
Copy link
Member Author

As of 20220727 we need to actively setup a PR for moving to NodeJS v16 there are multiple resources who now have a minimum of NodeJS v14.x.x

@cdcabrera cdcabrera mentioned this pull request Jul 27, 2022
Merged
cdcabrera added a commit that referenced this pull request Aug 1, 2022
@cdcabrera
fix(build): sw-460 npm updates (#958)
dfaa4a6 
* build, npm updates
* build, deps script, reset modules
* testing, latest jest, config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
202208 project phase build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cdcabrera @codecov-commenter