-
Notifications
You must be signed in to change notification settings - Fork 178
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
[RHOAIENG-4385] DSPA - Add back the Folder Info #2843
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2843 +/- ##
=======================================
Coverage 78.47% 78.47%
=======================================
Files 1127 1127
Lines 23955 23958 +3
Branches 6042 6044 +2
=======================================
+ Hits 18798 18802 +4
+ Misses 5157 5156 -1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Hi @jpuzz0, is there a mockup that I can refer to? |
Hey @DaoDaoNoCode, yes, this is the design, but it has changed a few times over the past couple days. Today some of the bold text has been updated, so I will address that, and there is still an outstanding question about the "Elyra" bit at the end. I would consider just the means in which the popover, general presentation, etc to be reviewable right now, but the content text itself is still up in the air slightly. |
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
This could be merged after the text is decided.
This is good to re-review @DaoDaoNoCode |
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.
Verified
/lgtm
FYI, I spoke with @jgarciao today and he confirmed the usage of the cloud service link for "Learn more". |
frontend/src/concepts/pipelines/content/configurePipelinesServer/ObjectStorageSection.tsx
Outdated
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/configurePipelinesServer/const.ts
Show resolved
Hide resolved
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DaoDaoNoCode, Gkrumbach07 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Closes: RHOAIENG-4385
Description
Added info alert with popover for Bucket field in the Configure Pipeline modal
(cc @yannnz)Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main