-
Notifications
You must be signed in to change notification settings - Fork 35
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
Skuba Patches #1274
Skuba Patches #1274
Conversation
🦋 Changeset detectedLatest commit: bdbe4dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -7,11 +7,10 @@ Processed 4 files in <random>s. | |||
|
|||
Prettier | |||
Processed 8 files in <random>s. | |||
Formatted 4 files: | |||
Formatted 3 files: |
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.
Gets formatted by the upgrade so it doesn't appear here
@@ -165,6 +165,6 @@ | |||
"entryPoint": "src/index.ts", | |||
"template": null, | |||
"type": "package", | |||
"version": "4.0.0" | |||
"version": "7.3.1" |
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.
This may or may not be bumped up in the version packages PR. (I didn't change this)
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.
This is the nested skuba.version
prop so I imagine it's not implicated in npm package versioning
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.
Also, how does lint calling upgradeSkuba work - would it still write?
Technically yes, it already does this so it wouldn't be a functional change 😅 |
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.
Logging sounds good
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.
Brill!
@@ -165,6 +165,6 @@ | |||
"entryPoint": "src/index.ts", | |||
"template": null, | |||
"type": "package", | |||
"version": "4.0.0" | |||
"version": "7.3.1" |
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.
This is the nested skuba.version
prop so I imagine it's not implicated in npm package versioning
Co-authored-by: Ryan Ling <[email protected]>
Co-authored-by: Ryan Ling <[email protected]>
These are just internal tests, but they are harmless to tweak and this may make it more obvious that we are intentionally choosing `fs-extra`. #1274 (comment)
These are just internal tests, but they are harmless to tweak and this may make it more obvious that we are intentionally choosing `fs-extra`. #1274 (comment)
Brought up in a previous PR.
We need a plan to manage when we run patches as at the moment we are always running them when realistically they only ever need to be run once.
By updating the skuba manifes version we can essentially see what version they were previously on versus now.
The version number in the
patches
folder is thefrom
version so we don't need to know what the new version number will be.