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

fix(cli): Conditionally show nudge about upgrade version toml tags #8039

Merged
merged 2 commits into from
Apr 27, 2023

Conversation

Josh-Walker-GM
Copy link
Collaborator

Problem
The nudge to include versions inside the toml notifications.versionUpdates happens for both tags and specific semver versions. It does not make sense for specific versions to be included only tags.

Changes

  1. Only show for tags.

Ping
@Tobbe - as you highlighted

@Josh-Walker-GM Josh-Walker-GM added the release:fix This PR is a fix label Apr 11, 2023
@replay-io
Copy link

replay-io bot commented Apr 11, 2023

16 replays were recorded for 9d6347c.

image 0 Failed
image 16 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/1afffe68-315a-4d31-a915-a24fcdddd826>useAuth hook, auth redirects checks</a></li>
      <li><a href=https://app.replay.io/recording/14c0397c-8e13-4438-bea5-eb02ccfa91f7>Check that a specific blog post is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/cfa33b20-8040-438c-bd49-477e38150fce>Check that about is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/1aa00e11-c4bd-4577-8f6b-d2ecda533347>Check that homepage is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/58dc4066-31fc-4e6d-a621-7b409be2f05a>Check that meta-tags are rendering the correct dynamic data</a></li>
      <li><a href=https://app.replay.io/recording/c78b3494-8ea5-4602-a6ca-23d7a068863a>Check that you can navigate from home page to specific blog post</a></li>
      <li><a href=https://app.replay.io/recording/a43f0111-f6c8-4e11-b61d-20a42b019e60>Waterfall prerendering (nested cells)</a></li>
      <li><a href=https://app.replay.io/recording/528e5836-720a-493e-bc65-1b053c2d6ae2>RBAC: Admin user should be able to delete contacts</a></li>
      <li><a href=https://app.replay.io/recording/33f6d3ff-2f7a-4fb2-aff6-17d3b4bf5502>RBAC: Should not be able to delete contact as non-admin user</a></li>
      <li><a href=https://app.replay.io/recording/6bdc17ff-eab6-413b-9b13-9f7260080cd2>Smoke test with dev server</a></li>
      <li><a href=https://app.replay.io/recording/f893f8da-a377-42c4-a5a4-670ca61b1498>Smoke test with rw serve</a></li>
      <li><a href=https://app.replay.io/recording/df5b8c33-fce5-4113-b335-0d76730b71a4>Loads Cell mocks when Cell is nested in another story</a></li>
      <li><a href=https://app.replay.io/recording/0eb9c9f1-3261-48a6-8d0b-2f185c6eeb91>Loads Cell Stories</a></li>
      <li><a href=https://app.replay.io/recording/1983320a-4132-48b8-a5a8-45e1e360480b>Loads MDX Stories</a></li>
      <li><a href=https://app.replay.io/recording/20831d34-4ed4-4122-b038-4ecf571f8d5e>Mocks current user, and updates UI while dev server is running</a></li>
      

View test run on Replay ↗︎

@Tobbe
Copy link
Member

Tobbe commented Apr 11, 2023

Right now, to try the v5 RC you have to do yarn rw upgrade -t 5.0.0-rc.642 (because if you do -t rc you get a v4.5 rc). Even so, I think it would be nice to be notified about new RC releases. How much work would it be to make that happen? Or even better, if I do yarn rw upgrade -t X.Y.Z-rc.### it would be cool if I could only be notified about vX RCs.

@Josh-Walker-GM
Copy link
Collaborator Author

It shouldn't be too difficult to list all the available versions, running npm view @redwoodjs/core versions returns a full list of versions:

[
  '0.0.0-experimental-streaming.4',
  '0.0.0-experimental-streaming.5',
  '0.0.0-experimental-streaming.6',
  '0.0.1-alpha.15',
  '0.0.1-alpha.16',
  '0.0.1-alpha.18',
  ...
  '5.0.0-canary.644',
  '5.0.0-canary.645',
  '5.0.0-canary.647',
  '5.0.0-canary.648',
  '5.0.0-rc.625',
  '5.0.0-rc.642',
  '5.0.0-rc.646'
]

The difficulty might come from collectively deciding on a fixed list of rules about what new version(s) to notify about based on the current version. Otherwise I imagine it should be a relatively basic mix of string processing, filtering the versions list and using semver comparisons.

@thedavidprice thedavidprice self-requested a review April 26, 2023 23:31
@thedavidprice
Copy link
Contributor

@Josh-Walker-GM Let's get this one in as-is. I like your ideas re: seeing a list of available versions, but it might be solved even more simply with a prompt + confirmation:

  1. yarn rw upgrade -t rc
  2. Console: $ about to upgrade to "5.0.0-rc.1": ok (y/n)?
  3. If no, show link to npm to see all available versions

Or something like that... At least we'd give better visibility into possible version issues before running the upgrade.

Regardless, this is for a follow-on PR (if at all).

Right now, to try the v5 RC you have to do yarn rw upgrade -t 5.0.0-rc.642 (because if you do -t rc you get a v4.5 rc).

^^ @Tobbe agreed this was super annoying. It's also an edge case we can manage better via process the next time around.

Thanks, gents! Getting this one in.

@thedavidprice thedavidprice merged commit a7879ab into main Apr 27, 2023
@thedavidprice thedavidprice deleted the jgmw-upgrade-toml-tag-nudge branch April 27, 2023 00:09
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Apr 27, 2023
@thedavidprice thedavidprice modified the milestones: next-release, v5.0.0 Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants