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

Platform Wide PF update: add dependabot #69

Open
wants to merge 13 commits into
base: master
from

Conversation

@carlosthe19916
Copy link
Contributor

carlosthe19916 commented Nov 7, 2019

https://projects.engineering.redhat.com/browse/RHCLOUD-3305

Enables Dependabot [1] in our application. Once merged, Dependabot will check for package version change and create PRs. It will make easier for us to use new package versions. It was also added to https://github.com/RedHatInsights/insights-frontend-starter-app/blob/master/.dependabot/config.yml so I think it might be a good idea.

Dependabot won't work unless the changes in this PR are in the master branch.

[1] https://app.dependabot.com/

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 7, 2019

Codecov Report

Merging #69 into master will decrease coverage by 0.24%.
The diff coverage is 64.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage   72.49%   72.24%   -0.25%     
==========================================
  Files          35       35              
  Lines         869      872       +3     
  Branches      169      172       +3     
==========================================
  Hits          630      630              
- Misses        218      220       +2     
- Partials       21       22       +1
Impacted Files Coverage Δ
...tionalComponents/ReportViewPage/ReportViewPage.tsx 89.74% <ø> (ø) ⬆️
src/actions/ReportActions.tsx 88.23% <ø> (ø) ⬆️
src/pages/ReportView/ReportView.js 95.65% <ø> (ø) ⬆️
src/api/report.tsx 53.33% <0%> (-2.81%) ⬇️
...ctCostBreakdownTable/ProjectCostBreakdownTable.tsx 100% <100%> (ø) ⬆️
src/pages/Reports/Reports.tsx 62.85% <100%> (ø) ⬆️
...onalComponents/FancyChartDonut/FancyChartDonut.tsx 68.18% <33.33%> (ø) ⬆️
src/pages/ReportsUpload/ReportsUpload.tsx 40.62% <50%> (ø) ⬆️

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 8ea40ef...1dbbd82. Read the comment docs.

@carlosthe19916 carlosthe19916 changed the title Add dependabot Platform Wide PF update: add dependabot Nov 13, 2019
@carlosthe19916 carlosthe19916 requested a review from mrizzi Nov 14, 2019
Copy link

mturley left a comment

Looks good to me 👍

@@ -87,7 +87,7 @@ exports[`ReportViewPage expect to render tabs when fetch status is complete 1`]
<PageHeaderTitle
title={
<React.Fragment>
<Breadcrumb
<Unknown

This comment has been minimized.

Copy link
@mturley

mturley Nov 18, 2019

I wonder what's causing these snapshots to render "Unknown" instead of some of the actual component names. Definitely not a big deal or a blocker, but I'm curious if it's something we could fix here or if it would need to be fixed in patternfly-react.

This comment has been minimized.

Copy link
@carlosthe19916

carlosthe19916 Nov 18, 2019

Author Contributor

@mturley Thank you very much for spending some time reviewing this PR, I really appreciate it :)... regarding this <Unknown> tag, it seems to me that it comes from patternfly-react, take a look to this link for instance: https://github.com/patternfly/patternfly-react/blob/6a11090171c69209a771b3ef1685377dc0f3aa71/packages/patternfly-4/react-core/src/components/OptionsMenu/__snapshots__/OptionsMenu.test.js.snap#L37 and the same for the <Component tag https://github.com/patternfly/patternfly-react/blob/6a11090171c69209a771b3ef1685377dc0f3aa71/packages/patternfly-4/react-core/src/layouts/Toolbar/__snapshots__/Toolbar.test.tsx.snap#L4 do you know if it the expected behaviour or should we create a new issue for that?

This comment has been minimized.

Copy link
@mturley

mturley Nov 18, 2019

Let's create an issue on patternfly-react for that. I think it's very low priority but snapshots should reflect the real display name of a component. It looks to me like since OptionsMenu now renders a Context.Provider at the top level, enzyme can't figure out what the actual component is for some reason. My guess is that setting a displayName property on the component before exporting it would fix this.

This comment has been minimized.

Copy link
@mturley

mturley Nov 18, 2019

Looks like there are others with this issue, it was brought up on PatternFly slack. I'll chime in there.

This comment has been minimized.

Copy link
@mturley

mturley Nov 19, 2019

Issue is open now: patternfly/patternfly-react#3317 and I think I have a fix for it

@@ -2,26 +2,15 @@

exports[`Environment expect to render 1`] = `
<Fragment>
<Table
<Component

This comment has been minimized.

Copy link
@mturley

mturley Nov 18, 2019

Same here with "Component" instead of "Table"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.