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

chore: move CLI exp setup to exp setup-opentelemetry #8256

Merged
merged 6 commits into from
May 8, 2023

Conversation

thedavidprice
Copy link
Contributor

I'm flattening the command structure exp setup [command] to improve discoverability. This is part 1 of a few more steps coming, which will make handling experimental packages and setup more feasible.

@Josh-Walker-GM Please do review and let me know your thoughts. NOTE: the Forum post instructions will need updating. I'm happy to do so — just let me know: https://community.redwoodjs.com/t/opentelemetry-support-experimental/4772

@thedavidprice thedavidprice added the release:chore This PR is a chore (means nothing for users) label May 8, 2023
@replay-io
Copy link

replay-io bot commented May 8, 2023

19 replays were recorded for e78b1d9.

image 0 Failed
image 19 Passed
    requireAuth graphql checks
          ```
          locator.waitFor: Target closed
          =========================== logs ===========================
          waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
          ============================================================
          ```
        </ol>
      </details>
      <li><a href=https://app.replay.io/recording/22ab372b-06db-45dc-b6aa-88de24f55299>useAuth hook, auth redirects checks</a></li>
      <li><a href=https://app.replay.io/recording/4275ff1a-24c7-459d-a07c-f0e1d8846878>Check that a specific blog post is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/fbc6a224-63a5-4856-a644-6ae6b919300c>Check that about is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/7d71e7ea-9381-45be-93e4-174b2b103b52>Check that homepage is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/adfb2d24-21d0-421b-baa4-2b72d31836a4>Check that meta-tags are rendering the correct dynamic data</a></li>
      <li><a href=https://app.replay.io/recording/ca24f97a-8a25-483b-bc73-28027a9cb0e7>Check that rehydration works for page not wrapped in Set</a></li>
      <li><a href=https://app.replay.io/recording/3891cae3-1032-42f7-b968-fa5a6a553fdf>Check that rehydration works for page with Cell in Set</a></li>
      <li><a href=https://app.replay.io/recording/39c28533-a000-465d-8954-e1b764c44214>Check that rehydration works for page with code split chunks</a></li>
      <li><a href=https://app.replay.io/recording/f9cf44be-063b-4861-a253-2af7edeb6360>Check that you can navigate from home page to specific blog post</a></li>
      <li><a href=https://app.replay.io/recording/bed4a2ab-7c0f-48bc-aafd-0097482b7316>Waterfall prerendering (nested cells)</a></li>
      <li><a href=https://app.replay.io/recording/0409063e-5860-400b-9b54-071a89e6c1ea>RBAC: Admin user should be able to delete contacts</a></li>
      <li><a href=https://app.replay.io/recording/594d483b-308d-4175-a074-919684937734>RBAC: Should not be able to delete contact as non-admin user</a></li>
      <li><a href=https://app.replay.io/recording/71b20210-4a2f-4e88-91fd-f558e613d2f8>Smoke test with dev server</a></li>
      <li><a href=https://app.replay.io/recording/411966b2-e3f9-475b-b628-cd425b4b6b99>Smoke test with rw serve</a></li>
      <li><a href=https://app.replay.io/recording/74738dce-6fb8-45c6-b81d-14415ddee64f>Loads Cell mocks when Cell is nested in another story</a></li>
      <li><a href=https://app.replay.io/recording/3bad5bdf-a290-4ce4-82e7-b6e1660ed339>Loads Cell Stories</a></li>
      <li><a href=https://app.replay.io/recording/107f881e-0b41-45d0-9e67-e8d7f0b7cf1d>Loads MDX Stories</a></li>
      <li><a href=https://app.replay.io/recording/bce45a12-b8a7-4b61-97b9-5672f99a7063>Mocks current user, and updates UI while dev server is running</a></li>
      

View test run on Replay ↗︎

@Josh-Walker-GM
Copy link
Collaborator

I'll review now. I didn't have any high-level plans when I did the initial setup of the exp CLI option. I only tried to make it as top level and so generic as possible. Imagining exp could prefix any of the existing cli command types, e.g. setup or generate. I don't have any strong opinions on structing it a particular way.

@thedavidprice
Copy link
Contributor Author

I only tried to make it as top level and so generic as possible. Imagining exp could prefix any of the existing cli command types, e.g. setup or generate. I don't have any strong opinions on structing it a particular way.

Roger that. This is meant to be iterative. And I think we can resolve better overall via a different approach — tbd when we meet this week.

Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM left a comment

Choose a reason for hiding this comment

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

Okay, I'll update the forum post if you like. Looks to me like the only change needed is to the command name?

Copy link
Contributor Author

Correct! And thank you

@thedavidprice thedavidprice enabled auto-merge (squash) May 8, 2023 21:29
@jtoar jtoar disabled auto-merge May 8, 2023 22:56
@jtoar jtoar merged commit 85c4d0d into main May 8, 2023
@jtoar jtoar deleted the dsp-remove-experimental-setup-command branch May 8, 2023 22:56
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone May 8, 2023
jtoar pushed a commit that referenced this pull request May 9, 2023
* move `exp setup` to `exp setup-opentelemetry`

* Add alias to docs

* fix commented-out packages

---------

Co-authored-by: Josh-Walker-GM <56300765+Josh-Walker-GM@users.noreply.github.com>
@jtoar jtoar modified the milestones: next-release, v5.1.0 May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants