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.

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

fix: re-add snyk code support, fixing cli executables #1691

Merged
merged 1 commit into from
Mar 7, 2021

Conversation

ArturSnyk
Copy link
Contributor

Reverts #1682

COD-123

  • Ready for review

What does this PR do?

This introduces an mpv usage for snyk code.

This is a second attempt:
the first pr failed due to es module issues in flatted from code-client->flat-cache->flatted
This issue has been resolved in code client and we now have a custom implementation of flat-cache specifically for our needs.
A second issue the arose dcignore was not included into executable at compilation stage. dcignore was the deepcode ignore package that is leveraged in the vscode extension through the code-client. However its future is uncertain, I have now raised a discussion around its future that will be decided on in the next week, the most likely outcome will be removing it from the code-client and re-writing the package and only using it through vs-code extension and if needed in the cli using it also directly in the cli.
I have temporarily fixed this by adding it to the included assets of pkg seen on the change in package json.

Where should the reviewer start?

you should have the snykcode cli's ff,
and run it with snyk code test or snyk code test <project_path>

How should this be manually tested?

snyk code test or snyk code test <project_path>

Any background context you want to provide?

we will be adding more functionality around this flow, more error handling, analytics, and output functionality, later on

What are the relevant tickets?

https://snyksec.atlassian.net/browse/COD-123

Screenshots

snyk code

@ArturSnyk ArturSnyk requested review from a team as code owners March 7, 2021 07:54
@ArturSnyk ArturSnyk self-assigned this Mar 7, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2021

Warnings
⚠️

Looks like you added a new Tap test. Consider making it a Jest test instead. See files like test/*.spec.ts for examples. Files found:

  • test/fixtures/sast/sample-analyze-folders-response.json
  • test/fixtures/sast/sample-sarif.json
  • test/fixtures/sast/test-output-windows.txt
  • test/fixtures/sast/test-output.txt

Generated by 🚫 dangerJS against 7881dbc

@ArturSnyk ArturSnyk force-pushed the revert-1682-revert/snyk-code branch from 0cbb543 to 223f364 Compare March 7, 2021 08:19
@ArturSnyk ArturSnyk changed the title Revert "Revert "Merge pull request #1664 from snyk/feat/add-snyk-code-as-plugin-for-test"" fix: re-add snyk code support, fixing cli executables Mar 7, 2021
@ArturSnyk ArturSnyk force-pushed the revert-1682-revert/snyk-code branch from 223f364 to 0d918a4 Compare March 7, 2021 08:22
@ArturSnyk ArturSnyk force-pushed the revert-1682-revert/snyk-code branch from 0d918a4 to 7881dbc Compare March 7, 2021 08:23
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2021

Expected release notes (by @ArturSnyk)

fixes:
re-add snyk code support, fixing cli executables (7881dbc)

others (will not be included in Semantic-Release notes):
avoid running package tests in root tests (1b2c741)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

@@ -2,5 +2,6 @@
"API": "https://snyk.io/api/v1",
"devDeps": false,
"PRUNE_DEPS_THRESHOLD": 40000,
"MAX_PATH_COUNT": 1500000
"MAX_PATH_COUNT": 1500000,
"CODE_CLIENT_PROXY_URL": "https://deeproxy.snyk.io"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adrukh 🤗

@@ -2,5 +2,6 @@
"API": "https://snyk.io/api/v1",
"devDeps": false,
"PRUNE_DEPS_THRESHOLD": 40000,
"MAX_PATH_COUNT": 1500000
"MAX_PATH_COUNT": 1500000,
"CODE_CLIENT_PROXY_URL": "https://deeproxy.snyk.io"
Copy link
Contributor

Choose a reason for hiding this comment

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

Awaiting our Slack discussion :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants