feat: add fake code scanning function [IDE-158] #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
teodora-sandu
force-pushed
the
feat/add-fake-code-scanning-function
branch
from
51f574c to
f422152
Compare
cmars
approved these changes
Feb 29, 2024
Contributor
cmars
left a comment
cmars
left a comment
There was a problem hiding this comment.
Really excited to see this starting! Some comments below to consider but nothing blocking.
Some ideas for followups:
- Could we generate the client (or just these wireformat types) from an OpenAPI spec? I've had good results with https://github.com/deepmap/oapi-codegen. This will help later when we need to iterate on new APIs, or support multiple versions.
- Could we introduce a linter (https://golangci-lint.run/) and add it to CI? I think it'll really help us maintain code quality in the long term.
Thanks again for this!
pkg/fake/fake.go
Outdated
| * limitations under the License. | ||
| */ | ||
|
|
||
| package fake |
Contributor
There was a problem hiding this comment.
Should this fake go into an internal package?
Contributor
Author
There was a problem hiding this comment.
The plan is to be able to use everything in this package from snyk-ls until Analysis have APIs for us to use. So the reason both this and the SARIF are exposed externally is because of that
| "encoding/json" | ||
| ) | ||
|
|
||
| var fakeResponse = `{ |
Contributor
There was a problem hiding this comment.
Consider loading this from a testdata fixture file (thinking of https://dave.cheney.net/2016/05/10/test-fixtures-in-go). It may be easier to work with (read, query, format, add more examples) as separate file(s).
bastiandoetsch
approved these changes
Mar 6, 2024
cat2608
reviewed
Mar 6, 2024
teodora-sandu
force-pushed
the
feat/add-fake-code-scanning-function
branch
from
5ffe93b to
e61a3ce
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adding a fake function that we can call from
snyk-lsand gradually add more functionality into. The reason I decided to return SARIF is because both the CLI andsnyk-lsread SARIF at the moment and format it into whatever they need to process. SARIF is what we will continue using for Snyk Code scanning as well, so it seemed like a safe first piece of "functionality" to add.Gradually, we will be moving the bundle creation and analysis code into here.