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.

Sign up for GitHub

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

Jump to bottom

Use allow/deny list in docs #1699

Closed
wants to merge 55 commits into from
Closed

Use allow/deny list in docs #1699

wants to merge 55 commits into from

Conversation

luciomartinez
Copy link
Contributor

Update documentation to use allowlist and denylist terms.

claudiahdz and others added 30 commits June 24, 2020 16:51
@claudiahdz
chore(docs): fixed links to cli commands
2e05298 
This reverts commit 0eac801.

PR-URL: #1459
Credit: @claudiahdz
Close: #1459
Reviewed-by: @claudiahdz
@peterfich @claudiahdz
Update npm-link.md
0ca3509 
Fix typo.

PR-URL: #1283
Credit: @peterfich
Close: #1283
Reviewed-by: @claudiahdz
@maxwellgerber @claudiahdz
docs: Add note about dropped * filenames
3dd429e 
In npm/npm-packlist#32, NPM started to drop filenames containing the `*` character.

See:
- #1096
- #1048

PR-URL: #1377
Credit: @maxwellgerber
Close: #1377
Reviewed-by: @claudiahdz
@kemitchell @claudiahdz
spdx-license-ids@3.0.5
c49b6ae 
PR-URL: #1418
Credit: @kemitchell
Close: #1418
Reviewed-by: @claudiahdz
@seanpoulter @claudiahdz
docs: Fix typo
9a2e2e7 
PR-URL: #1429
Credit: @seanpoulter
Close: #1429
Reviewed-by: @claudiahdz
@vanishcode @claudiahdz
fix: wrong npm doctor command result
b7ad775
@vanishcode @claudiahdz
test: add test for npm doctor that ping registry returns error
1aec4cb
@vanishcode @claudiahdz
style: fix lint error with no trailing comma
479e45c 
PR-URL: #1416
Credit: @vanishcode
Close: #1416
Reviewed-by: @claudiahdz
@claudiahdz
chore: remove auth info from logs
a9857b8
@claudiahdz
npm-registry-fetch@4.0.5
94eca63
@claudiahdz
docs: changelog for 6.14.6
66aab41
@claudiahdz
update AUTHORS
46e91d9
@claudiahdz
6.14.6
66092d5
@claudiahdz
libnpx@10.2.4
6a6151f
@claudiahdz
deps: update mkdirp on tacks
2d78481
@claudiahdz
deps: uninstall npm-registry-couchapp
4e129d1
@claudiahdz
deps: fix outdated dev deps
8e1869e
@jasisk @claudiahdz
fix: npm explore spawn shell correctly
de51088 
PR-URL: #784
Credit: @jasisk
Close: #784
Reviewed-by: @claudiahdz
@claudiahdz
fix: git tag handling regression on shrinkwrap
36e6c01
@bz2 @claudiahdz
Fix package id in shrinkwrap lifecycle step output
1961c93 
Currently all logging related to shrinkwrap steps reports 'undefined'
for the package in output and log messages.

This is due to the package associated with the `idealTree` being
recreated in the `savePackageJson()` method which precedes these
steps. For now, just copy forward the `_id` attribute which lifecycle
logging expects, but note that mutating `package` here is surprising.

Fixes npm/npm#20756

PR-URL: #288
Credit: @bz2
Close: #288
Reviewed-by: @claudiahdz
@claudiahdz
fix: gracefully handle error during npm install
8788889 
PR-URL: #1009
Credit: @danielleadams
Close: #1009
Reviewed-by: @claudiahdz
@ruyadorno @claudiahdz
fix: npm ls --parseable --long output
6fe2bdc 
PR-URL: #1547
Credit: @ruyadorno
Close: #1547
Reviewed-by: @claudiahdz
@claudiahdz
bin-links@1.1.8
dc21422
@claudiahdz
gentle-fs@2.3.1
d341f88
@claudiahdz
libcipm@4.0.8
3e168d4
@claudiahdz
npm-audit-report@1.3.3
6ae942a
@claudiahdz
npm-lifecycle@3.1.5
6a35e3d
@claudiahdz
docs: changelog for 6.14.7
e6e8d29
@claudiahdz
update AUTHORS
a81da2b
@claudiahdz
6.14.7
42e4625
@luciomartinez luciomartinez requested a review from a team as a code owner August 21, 2020 21:06
ruyadorno
ruyadorno suggested changes Aug 27, 2020
View reviewed changes
Copy link
Contributor

@ruyadorno ruyadorno left a comment

Choose a reason for hiding this comment

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

awesome! thanks for the contribution @luciomartinez ❤️

I find that these phrases need a better construction rather than just replacing the words there but again, thank you so much for bringing this up. We'll make sure to include this in the next release as soon as the requested changes are addressed. 😊

docs/content/configuring-npm/package-json.md Outdated Show resolved Hide resolved
docs/content/configuring-npm/package-json.md Outdated Show resolved Hide resolved
docs/content/configuring-npm/package-json.md Outdated Show resolved Hide resolved
docs/content/using-npm/developers.md Outdated Show resolved Hide resolved
luciomartinez and others added 4 commits August 28, 2020 17:58
@luciomartinez @ruyadorno
Update phrase
5db2908 
Co-authored-by: Ruy Adorno <ruyadorno@github.com>
@luciomartinez @ruyadorno
Update phrase
2f1ce13 
Co-authored-by: Ruy Adorno <ruyadorno@github.com>
@luciomartinez @ruyadorno
Update phrase
ba6f2ee 
Co-authored-by: Ruy Adorno <ruyadorno@github.com>
@luciomartinez @ruyadorno
Update phrase
75cd049 
Co-authored-by: Ruy Adorno <ruyadorno@github.com>
@luciomartinez
Copy link
Contributor Author

@ruyadorno cool! I was thinking to wrap the terms within code blocks to stand out but this is much better, keeping a more readable grammar. Thanks a lot

@pkrasicki
Copy link

I just noticed this PR and the original text is much easier to understand for me. It seems that everyone knows what words whitelist and blacklist mean. Why did you decide to replace them with more words that don't fit so well in this context?

@ljharb
Copy link
Contributor

ljharb commented Aug 29, 2020

What they mean tho is “allow list” and “block list”, so this is actually clearer and a much better fit.

@pkrasicki
Copy link

pkrasicki commented Aug 29, 2020 •
edited
Loading

Perhaps, but compare those two sentences:
You can also blacklist instead of whitelist operating systems - this is the original
You can also block list instead of allow list operating systems - replacement

The second one doesn't make any sense, which is why author of this PR had to add more changes.

@ljharb
Copy link
Contributor

ljharb commented Aug 29, 2020

Sure, the verb usage is different, but these are still better terms and worth the additional changes.

@darcyclarke darcyclarke added the Release 6.x work is associated with a specific npm 6 release label Sep 1, 2020
@darcyclarke darcyclarke changed the base branch from latest to release/v7.0.0-beta September 18, 2020 18:30
@darcyclarke darcyclarke added Release 7.x work is associated with a specific npm 7 release semver:patch semver patch level for changes and removed Release 6.x work is associated with a specific npm 6 release labels Sep 18, 2020
ruyadorno pushed a commit that referenced this pull request Sep 22, 2020
@ruyadorno
Use allow/deny list in docs
5837a48 
PR-URL: #1699
Credit: @luciomartinez
Close: #1699
Reviewed-by: @ruyadorno
@ruyadorno ruyadorno mentioned this pull request Sep 22, 2020
Merged
@ruyadorno
Copy link
Contributor

Landed in 5837a48

@ruyadorno ruyadorno closed this Sep 22, 2020
@luciomartinez luciomartinez deleted the allow-deny-list branch September 23, 2020 09:34
@Omrisnyk Omrisnyk mentioned this pull request Nov 27, 2023
Open
@Omrisnyk Omrisnyk mentioned this pull request Dec 20, 2023
Open
@Omrisnyk Omrisnyk mentioned this pull request Feb 12, 2024
Open
@Omrisnyk Omrisnyk mentioned this pull request Mar 22, 2024
Open
@Omrisnyk Omrisnyk mentioned this pull request May 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruyadorno ruyadorno ruyadorno approved these changes

Assignees
No one assigned
Labels
Release 7.x work is associated with a specific npm 7 release semver:patch semver patch level for changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.