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

Additional Messaging for Compatibility Checking for Create Redwood App #8673

Merged

Conversation

ahaywood
Copy link
Contributor

@ahaywood ahaywood commented Jun 20, 2023

Notion ID: CTL-130

Feature description

  • Created a How To Guide for Working with nvm
  • When the user needs to upgrade node, I added additional messaging and linked to the How To Use nvm guide
  • When the user needs to downgrade node, I added additional messaging and linked to the How to Use nvm guide.

Output Screenshots

If the user is above the supported version of node:
CleanShot 2023-06-20 at 17 33 46@2x

If the user is below the supported version of node:
CleanShot 2023-06-20 at 17 37 42@2x

@ahaywood ahaywood requested a review from jtoar June 20, 2023 23:12
@ahaywood ahaywood added the topic/crwa create-redwood-app label Jun 20, 2023
@notion-workspace
Copy link

Compatibility Check Error

@jtoar jtoar added the release:feature This PR introduces a new feature label Jun 20, 2023
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

Hey @ahaywood, I haven't looked at the docs yet but there's a small regression here in that we no longer handle the case when yarn is below the required version:

image

I set yarn to 1.14.0 via

yarn set version 1.14.0 # for the framework
corepack prepare yarn@1.14.0 --activate # for my system

The errorMessages const handled the yarn error messaging:

const errorMessages = [
{
type: 'node',
failedCompatibilityCheck: foundNodeVersionIsLessThanRequired,
},
{
type: 'yarn',
failedCompatibilityCheck: foundYarnVersionIsLessThanRequired,
},
]
.filter(({ failedCompatibilityCheck }) => failedCompatibilityCheck)
.map(
({ type }) =>
` ${type} ${checksData[type].wanted.range} required; found ${checksData[type].version.version}`
)

We probably just need another conditional somewhere to show the right error.

@ahaywood
Copy link
Contributor Author

@jtoar I added an additional conditional to check for the user's current version of yarn. I also created a doc specifically for yarn. This is ready for another review.

@jtoar jtoar added this to the v5.4.0 milestone Jun 30, 2023
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

@ahaywood the code changes work great! Just reviewed the docs and they look good too, just left a few minor comments. We just need to get rid of the lint error by removing the unused variable errorMessages variables and CI should pass.

docs/docs/how-to/using-nvm.md Outdated Show resolved Hide resolved
docs/docs/how-to/using-nvm.md Outdated Show resolved Hide resolved
docs/docs/how-to/using-nvm.md Outdated Show resolved Hide resolved
docs/docs/how-to/using-yarn.md Outdated Show resolved Hide resolved
docs/docs/how-to/using-yarn.md Outdated Show resolved Hide resolved
@jtoar jtoar modified the milestones: v5.4.0, next-release Jun 30, 2023
@ahaywood
Copy link
Contributor Author

ahaywood commented Jul 5, 2023

@jtoar Made updates and ready for your review (again)

@ahaywood ahaywood requested a review from jtoar July 5, 2023 22:12
jtoar pushed a commit that referenced this pull request Jul 6, 2023
**Notion ID:**
[CTL-282](https://www.notion.so/redwoodjs/Add-documentation-for-all-the-prompts-that-are-included-aaaab2be79cf4bb69dbf3962a5bd03a8?pvs=4)

## Feature description

Added documentation for the Create Redwood App. This doc includes
- All the prompts that the Create Redwood App wil ask you. 
- Instructions for running some of the commands manually
- Information about all the flags available

## Related PRs: 
#8673 
includes a doc for working with nvm. I linked to the doc created within
that PR.
jtoar pushed a commit that referenced this pull request Jul 6, 2023
**Notion ID:**
[CTL-282](https://www.notion.so/redwoodjs/Add-documentation-for-all-the-prompts-that-are-included-aaaab2be79cf4bb69dbf3962a5bd03a8?pvs=4)

## Feature description

Added documentation for the Create Redwood App. This doc includes
- All the prompts that the Create Redwood App wil ask you. 
- Instructions for running some of the commands manually
- Information about all the flags available

## Related PRs: 
#8673 
includes a doc for working with nvm. I linked to the doc created within
that PR.
@jtoar jtoar merged commit 1a78c0a into redwoodjs:main Jul 6, 2023
7 of 12 checks passed
@jtoar jtoar modified the milestones: next-release, v6.0.0 Jul 6, 2023
jtoar added a commit that referenced this pull request Jul 6, 2023
#8673)

**Notion ID:**
[CTL-130](https://www.notion.so/redwoodjs/Compatibility-Check-Error-593a7c98815a4b2688f04a64cc989ea2?pvs=4)

## Feature description
- Created a How To Guide for Working with nvm
- When the user needs to upgrade node, I added additional messaging and
linked to the How To Use nvm guide
- When the user needs to downgrade node, I added additional messaging
and linked to the How to Use nvm guide.

## Output Screenshots
If the user is above the supported version of node:
<img width="837" alt="CleanShot 2023-06-20 at 17 33 46@2x"
src="https://github.com/redwoodjs/redwood/assets/212300/4ee7f103-932e-4a0c-b36b-9c7fe4c963fd">

If the user is below the supported version of node:
<img width="836" alt="CleanShot 2023-06-20 at 17 37 42@2x"
src="https://github.com/redwoodjs/redwood/assets/212300/5c301822-b806-41cc-9a78-5a33fdff7582">

---------

Co-authored-by: Dominic Saadi <dominiceliassaadi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature topic/crwa create-redwood-app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants