Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs(website): Update tutorial #8737
base: main
Are you sure you want to change the base?
docs(website): Update tutorial #8737
Changes from all commits
c359d20
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Note that it might make sense to stick to the older 18 patch-level for comparability to some "mime-types-2.1.18" test assets we have in our code base, like https://github.com/oss-review-toolkit/ort/blob/main/scanner/src/test/assets/scancode-3.0.2_mime-types-2.1.18.json.
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 updated because using
2.1.18
resulted in tons of issues due to outdated dependencies. Which makes it harder to read the resulting file.2.1.35
has issues too, but (currently) only a few.Would it be worth to update the tests assets?
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.
What kind of issues exactly? Do you mean security vulnerabilities? However, I believe we do not even run the advisor yet as part of the tutorial... see #4219.
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.
a) Why 4x the same warning?
b) Should this be warning just be left out of the tutorial, or addressed by some other means?
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.
Because all of NPM, PNPM, YARN, YARN2 individually state via common
NpmDetection
code that none of them is clearly responsible for that definition file.If quoting console output, I believe it should be complete to avoid confusion about differences. Maybe the only exception should be lengthy / unimportant output that could be replaced by ellipsis (
[...]
).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.
Would be nice to have an explanation here, but I do not understand this yet.
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.
You mean the "deprecated [email protected]: Glob versions prior to v9 are no longer supported" message? That's just being forwarded from NPM itself. Apparently the project uses an unsupported / unmaintained version of the
glob
package.