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

Use tags from API instead of mocked ones #278

Merged
merged 3 commits into from
Nov 22, 2019

Conversation

karelhala
Copy link
Collaborator

@karelhala karelhala commented Nov 18, 2019

@karelhala karelhala added the on hold Unfinished work - DO NOT MERGE PR label Nov 18, 2019
@codecov-io
Copy link

codecov-io commented Nov 18, 2019

Codecov Report

Merging #278 into master will increase coverage by 0.04%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #278      +/-   ##
==========================================
+ Coverage   59.29%   59.34%   +0.04%     
==========================================
  Files         166      166              
  Lines        2501     2499       -2     
  Branches      457      457              
==========================================
  Hits         1483     1483              
+ Misses        848      846       -2     
  Partials      170      170
Impacted Files Coverage Δ
packages/inventory/src/EntityTable.js 0% <ø> (ø) ⬆️
packages/inventory/src/redux/entityDetails.js 0% <ø> (ø) ⬆️
packages/inventory/src/EntityDetail.js 0% <ø> (ø) ⬆️
packages/inventory/src/redux/actions.js 0% <0%> (ø) ⬆️
packages/inventory/src/redux/entities.js 0% <0%> (ø) ⬆️
packages/inventory/src/TagsModal.js 0% <0%> (ø) ⬆️

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 8b6cdda...d14a0e3. Read the comment docs.

@karelhala karelhala changed the title [WIP] Use tags from API instead of mocked ones Use tags from API instead of mocked ones Nov 20, 2019
@karelhala karelhala added release Once merged it will trigger bugfix release enhancement New feature or request and removed on hold Unfinished work - DO NOT MERGE PR labels Nov 20, 2019
...data,
results: data.results.map(row => ({ ...row, tags: tags[row.id] || [] }))
}))
.catch(() => ({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is safe way how not to fail on fetching tags (they are something trivial and shouldn't break the UI)

@karelhala karelhala requested a review from a team November 20, 2019 15:29
Copy link

@yashnalla yashnalla left a comment

Choose a reason for hiding this comment

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

Everything looks good; in terms of functionality/code. I did notice when I was npm linking that this depends on repository.engineering.redhat.com. I thought we were using regular npm registry because we wanted community contributions or is that something only for insights-chrome?

@karelhala
Copy link
Collaborator Author

@yashnalla mhhh, that is trange. I can't seem to be able to find any reference to it in lock file https://raw.githubusercontent.com/RedHatInsights/frontend-components/d14a0e3b12899a3922cd5737445d3d46c7069d4c/package-lock.json

@karelhala karelhala merged commit 123c1ae into RedHatInsights:master Nov 22, 2019
@nacho-bot
Copy link
Collaborator

                      :soon::shipit::octocat:
     :bug:Shipit Squirrel has this release bugfix surrounded, be ready for a new version:beetle:

@karelhala karelhala deleted the use-api-tags branch November 22, 2019 08:43
@nacho-bot
Copy link
Collaborator

     🌱 🌸 🌷 🌻 🌟 New version of package has been released 🌟 🌻 🌷 🌸 🌱

                The release is available on:

       :package:@redhat-cloud-services/frontend-components-inventory-general-info/v/0.0.14📦

             :boom:This feature is brought to you by probot🚀

@nacho-bot nacho-bot added the released Pr has been released label Nov 22, 2019
@nacho-bot
Copy link
Collaborator

     🌱 🌸 🌷 🌻 🌟 New version of package has been released 🌟 🌻 🌷 🌸 🌱

                The release is available on:

        :package:@redhat-cloud-services/frontend-components-inventory/v/0.0.36📦

             :boom:This feature is brought to you by probot🚀

@yashnalla
Copy link

Yea i noticed that too; I didn't see it when I was npm installing; just npm linking. No idea why. Might just be some npm behavior I don't know about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request release Once merged it will trigger bugfix release released Pr has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants