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

ui: fix storybook for db-console and make it run with bazel #104259

Merged
merged 1 commit into from
Jul 10, 2023
Merged

ui: fix storybook for db-console and make it run with bazel #104259

merged 1 commit into from
Jul 10, 2023

Conversation

koorosh
Copy link
Collaborator

@koorosh koorosh commented Jun 2, 2023 •
edited
Loading

This change includes several changes that enables better experience for developers to build visual components with Storybook, now it is possible to run Storybook with dev ie:

dev ui storybook --project cluster-ui --port 9009
or
dev ui storybook # db-console project is default one

Release note: None

Epic: None

@koorosh koorosh requested a review from a team as a code owner June 2, 2023 15:02
@koorosh koorosh requested a review from a team June 2, 2023 15:02
@blathers-crl
Copy link

blathers-crl bot commented Jun 2, 2023

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added the O-community Originated from the community label Jun 2, 2023
@cockroach-teamcity
Copy link
Member

This change is Reviewable

gtr
gtr approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@gtr gtr left a comment

Choose a reason for hiding this comment

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

Fixture changes LGTM! The dev ui storybook subcommand changes also look good but perhaps a second set of eyes would be good on those changes.

Reviewed 6 of 8 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @j82w, @maryliag, @THardy98, @xinhaoz, and @zachlite)

@koorosh
Copy link
Collaborator Author

koorosh commented Jul 7, 2023

bors r+

maryliag
maryliag approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 8 of 8 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @gtr, @j82w, @THardy98, @xinhaoz, and @zachlite)

@craig
Copy link
Contributor

craig bot commented Jul 7, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Jul 7, 2023

Build failed (retrying...):

@yuzefovich
Copy link
Member

Seems like this PR needs a rebase:

[21:37:27][Step 4/4] Your branch is up to date with 'origin/staging'.
[21:37:27][Step 4/4] 
[21:37:27][Step 4/4] Changes not staged for commit:
[21:37:27][Step 4/4]   (use "git add <file>..." to update what will be committed)
[21:37:27][Step 4/4]   (use "git restore <file>..." to discard changes in working directory)
[21:37:27][Step 4/4] 	modified:   .aspect/rules/external_repository_action_cache/npm_translate_lock_LTY0NDkzMjUwNQ==
[21:37:27][Step 4/4] 	modified:   pkg/ui/workspaces/db-console/pnpm-lock.yaml
...

bors r-

@craig
Copy link
Contributor

craig bot commented Jul 7, 2023

Canceled.

@koorosh koorosh force-pushed the ui-storybook-with-bazel branch from 35097be to da6af95 Compare July 8, 2023 10:25
@blathers-crl
Copy link

blathers-crl bot commented Jul 8, 2023

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@koorosh koorosh force-pushed the ui-storybook-with-bazel branch from da6af95 to c16a0bd Compare July 8, 2023 16:21
@koorosh
Copy link
Collaborator Author

koorosh commented Jul 10, 2023

There was merge conflict in yarn.lock file. I had to reinstall deps once again and it requires to run dev ui mirror-deps.
Need someone (within CRL) to fetch this PR and run this command locally.

cockroach/pkg/ui/README.md

Lines 178 to 182 in 0207c61

### Mirroring and Rewriting yarn.lock
To upload new dependencies to Google Cloud Storage, you'll need to be a
Cockroach Labs employee signed into the `gcloud` CLI. Simply run
`./dev ui mirror-deps` from the root of `cockroach.git`, and any new
dependencies will be uploaded and all yarn.lock files rewritten:

@zachlite zachlite force-pushed the ui-storybook-with-bazel branch from c16a0bd to 1d0a199 Compare July 10, 2023 16:19
@koorosh
Copy link
Collaborator Author

koorosh commented Jul 10, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 10, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@koorosh
ui: fix storybook for db-console and make it run with bazel
550aad8 
This change includes several changes that enables better experience
for developers to build visual components with Storybook.

- fixed storybook configuration for Db Console project. It required to
install `core-js` package to resolve following issues: https://github.com/storybookjs/storybook/blob/main/MIGRATION.md#core-js-dependency-errors
- updated fixtures for cluster UI storybook that wasn't adjusted to changes
in components they rely on.
- Added new `dev ui storybook` subcommand that allows run storybooks for
Db Console or Cluster UI projects.

Release note: None
@koorosh koorosh force-pushed the ui-storybook-with-bazel branch from 1d0a199 to 550aad8 Compare July 10, 2023 20:50
@craig
Copy link
Contributor

craig bot commented Jul 10, 2023

GitHub status checks took too long to complete, so bors is giving up. You can adjust bors configuration to have it wait longer if you like.

@koorosh
Copy link
Collaborator Author

koorosh commented Jul 10, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 10, 2023

Build succeeded:

@craig craig bot merged commit 3ca7382 into cockroachdb:master Jul 10, 2023
@koorosh koorosh mentioned this pull request Jul 12, 2023
Merged
craig bot pushed a commit that referenced this pull request Jul 12, 2023
@yuzefovich @koorosh
Merge #106659 #106724
2dbec84 
106659: sql: fix pausable portal execution of apply joins with subqueries r=yuzefovich a=yuzefovich

Previously, when executing a query with an apply join with "inner" subqueries via pausable portals model we would crash because we'd incorrectly apply the "pausable portal" execution model to those "inner" subqueries (even though they need to be executed completely). We already disabled this execution model for the main "inner" query, but it should be disabled for the whole apply join iteration.

I decided to not include a release note given that pausable portals are disabled by default and apply joins with inner subqueries seem like an edge case.

Epic: None

Release note: None

106724: ui: revert dependency changes that broke `dev ui watch` command r=koorosh a=koorosh

This patch reverts part of the changes from #104259 that were made to run storybook with `dev` but introduced regression to `dev ui watch` command.

No one made changes in `package.json` and `yarn.lock` after me so it was safe to revert those files.
With this reverted change `dev ui storybook` command doesn't work which kind of fine since it is not 
used anywhere.

Release note: None

Epic: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Andrii Vorobiov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtr gtr gtr approved these changes

@maryliag maryliag maryliag approved these changes

@zachlite zachlite Awaiting requested review from zachlite

@j82w j82w Awaiting requested review from j82w

@THardy98 THardy98 Awaiting requested review from THardy98

@xinhaoz xinhaoz Awaiting requested review from xinhaoz

Assignees
No one assigned
Labels
O-community Originated from the community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@koorosh @cockroach-teamcity @yuzefovich @maryliag @gtr