-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat(RHINENG-8881): Add bootc card host detail page #2168
Conversation
fc8b754
to
fd43bfd
Compare
Hey @dkuc !
|
@computercamplove Yes, that is the correct mock. |
@dkuc the commits need to comply with the commit lint. The title should be similar to this: For more info: https://docs.google.com/document/d/1BsZJXN2ygd-dOAN6IEpkPp-oQUoh4nHbtFLre6D5JoM/edit#heading=h.rt93vzt2dkoh |
fyi main part is working as expected - this card in host's details i see only for image-mode systems, thank you! yet i still have some questions to clarify about card name and URL link - i left question in jira for Andy and Christopher https://issues.redhat.com/browse/RHINENG-8881. |
/retest |
got clarifications - this UI is expected for the first iteration (more info in card https://issues.redhat.com/browse/RHINENG-8881) fyi @mkholjuraev |
@@ -0,0 +1,56 @@ | |||
import React from 'react'; | |||
import PropTypes from 'prop-types'; | |||
import { connect } from 'react-redux'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although connect
function is still supported, it is recommended to use the hooks API in recent versions of Redux. In this functional component, mainly data is read from the redux store. Thus, useSelector hook may be used for this purpose. Let me know if I should help by implementing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkholjuraev Yes, help with this would be appreciated.
e6f194d
to
fb11528
Compare
/retest |
/retest |
FYI @mkholjuraev, @dkuc i added test specific for this PR, it passed. Ignore test_filter_hosts_by_os |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2168 +/- ##
==========================================
- Coverage 43.68% 43.04% -0.64%
==========================================
Files 198 199 +1
Lines 6286 6300 +14
Branches 1759 1764 +5
==========================================
- Hits 2746 2712 -34
- Misses 3540 3588 +48 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! tested with 'image-mode-test-60e1ea67' bootc system and other non bootc systems.
🎉 This PR is included in version 1.67.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Adds a new card with bootc system information. It is hidden when the system is not a bootc system.
Note to reviewers: Feel free to make changes directly. Close and reopen a new PR if needed. While I do believe this works and can be merged, its purpose is to imply intent.