-
Notifications
You must be signed in to change notification settings - Fork 169
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
Resolve failing test scenarios #580
Conversation
package.json
Outdated
@@ -109,10 +108,10 @@ | |||
} | |||
}, | |||
"engines": { | |||
"node": ">= 20" | |||
"node": ">=20.11.0" | |||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove the node engine from the package because in the consuming app it doesn't matter what version of node it's using
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I even leave the "engines" section in there at all then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no we don't need the engines section any more 👍
@mansona @knownasilya @ef4 I don't know if I can get farther than this without some help, would y'all take a look at the release/beta/canary failures and LMK what you think? TIA!! |
12096c1
to
09cba40
Compare
If there's no objections, I'd like to merge this PR, do a release, and then keep working on the release/beta/canary issues so we can at least have some incremental progress. |
If merged, this PR tries to resolve some scenarios and tidy up some inconsistencies (spelling errors, dependency versions, instructions, etc).
Currently, I can't seem to get release/beta/canary to work, but the embroider scenario now passing is an improvement