-
Notifications
You must be signed in to change notification settings - Fork 588
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
Respect per-package package.json publishConfig.access #204
Conversation
🦋 Changeset is good to goLatest commit: acc01ca We got this. 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 |
dc8fd1f
to
3db5e56
Compare
Codecov Report
@@ Coverage Diff @@
## master #204 +/- ##
==========================================
- Coverage 76.4% 76.34% -0.06%
==========================================
Files 41 41
Lines 1051 1057 +6
Branches 223 232 +9
==========================================
+ Hits 803 807 +4
- Misses 244 246 +2
Partials 4 4
Continue to review full report at Codecov.
|
packages/config/src/index.ts
Outdated
@@ -57,15 +57,15 @@ export let parse = ( | |||
|
|||
if ( | |||
json.access !== undefined && | |||
json.access !== "private" && | |||
json.access !== "restricted" && | |||
json.access !== "public" |
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 still allow access === "private"
so that the old default config isn't broken but let's normalize it to restricted and log a warning.
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.
ok, I've added handling of this
3db5e56
to
42f1ce4
Compare
42f1ce4
to
dd6f61e
Compare
I think this is good to merge. I'm tempted to merge it despite failing coverage goals, but would be great not to degrade coverage. lmk what you want to do. |
I'm afraid I won't be able to work on increasing the coverage in the following days. |
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.
I just pushed a test for the config stuff. The other coverage decrease is in the publish which we don't really have any tests for yet so that shouldn't really affect this PR
* Respect per-package package.json publishConfig.access * Normalize private access to restricted * Add a config test * Linting
fixes #193
closes #203
is combination of #203 and Andarist@82c83ba