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

feat(ws): Notebooks 2.0 // Frontend // Workspaces details // Activity tab #171 #199

Merged
merged 2 commits into from
Feb 17, 2025
Merged

feat(ws): Notebooks 2.0 // Frontend // Workspaces details // Activity tab #171 #199

merged 2 commits into from
Feb 17, 2025

Conversation

asaadbalum
Copy link

image

@asaadbalum asaadbalum changed the base branch from main to notebooks-v2 February 6, 2025 15:32
@google-oss-prow google-oss-prow bot added size/M and removed size/XXL labels Feb 6, 2025
paulovmr
paulovmr suggested changes Feb 6, 2025
View reviewed changes
Copy link

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

Hi @asaadbalum , thanks for the PR! LGTM, I've left two inline comments about the naming. Also, there are some formatting issues, that can be fixed by npm run test:fix.
Apart from that, the DCO check is failing on this PR.

@asaadbalum asaadbalum force-pushed the RHOAIENG-16287-2 branch 4 times, most recently from 13a4550 to 263118e Compare February 11, 2025 13:14
@paulovmr
Copy link

/ok-to-test

@paulovmr
Copy link

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Feb 12, 2025
paulovmr
paulovmr suggested changes Feb 12, 2025
View reviewed changes
Copy link

@paulovmr paulovmr left a comment

Choose a reason for hiding this comment

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

Hi @asaadbalum, there is one important thing to change: the style of the overview tab (WorkspaceDetailsOverview.tsx) was changed, so this PR should match it:
image

@google-oss-prow google-oss-prow bot removed the lgtm label Feb 13, 2025
@asaadbalum
Copy link
Author

Hi @asaadbalum, there is one important thing to change: the style of the overview tab (WorkspaceDetailsOverview.tsx) was changed, so this PR should match it: image

Modified

ederign
ederign suggested changes Feb 13, 2025
View reviewed changes
Copy link
Member

@ederign ederign left a comment

Choose a reason for hiding this comment

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

Hi @asaadbalum the code looks ok, but can you please introduce some tests for it? We need to improve our coverage.

@asaadbalum
feat(ws): Notebooks 2.0 // Frontend // Workspaces details // Activity…
5aa4763 
… tab kubeflow#171

Signed-off-by: Asaad Balum <asaad.balum@gmail.com>
@google-oss-prow google-oss-prow bot added size/L and removed size/M labels Feb 16, 2025
@asaadbalum asaadbalum force-pushed the RHOAIENG-16287-2 branch 3 times, most recently from 8f58ce2 to 9307cb5 Compare February 16, 2025 14:17
@asaadbalum
Merge branch 'notebooks-v2' into RHOAIENG-16287-2
63d96a2 
Signed-off-by: asaadbalum <154635253+asaadbalum@users.noreply.github.com>
Signed-off-by: Asaad Balum <asaad.balum@gmail.com>
@ederign
Copy link
Member

ederign commented Feb 17, 2025

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Feb 17, 2025
@ederign
Copy link
Member

ederign commented Feb 17, 2025

/approve

@google-oss-prow Google OSS Prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ederign, paulovmr

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 6216ab0 into kubeflow:notebooks-v2 Feb 17, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ederign ederign ederign requested changes

@paulovmr paulovmr paulovmr approved these changes

@kimwnasptd kimwnasptd Awaiting requested review from kimwnasptd

@thesuperzapper thesuperzapper Awaiting requested review from thesuperzapper

Assignees

@ederign ederign

@paulovmr paulovmr

Labels
approved lgtm ok-to-test size/L
Projects
Kubeflow Notebooks
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@asaadbalum @paulovmr @ederign