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

refactor: snyk code client improvements #11

Merged
merged 7 commits into from
Mar 15, 2024

Conversation

teodora-sandu
Copy link
Collaborator

Contains improvements discussed in #10, such as:

  • Refactoring the error reporter interface so that CaptureError takes in some "options" that tell it whether to send diagnostics for LSP or not
  • Using the configuration from go-application-framework instead of snyk-ls - for this the only "configuration" GAF doesn't have is the snykCodeApiUrl, which instead of moving over I decided to add as another function argument to the functions but I'm happy to discuss
  • Running golagnci-lint in fixing mode when make format runs
  • Using the logger from go-application-framework
  • Defining the encoder from snyk-ls in this repo so that we don't depend on snyk-ls (eventually the code in snyk-ls should just disappear, I'm unsure whether to refactor it now or not)

@teodora-sandu teodora-sandu changed the title Refactor/snyk code client refactor: snyk code client improvements Mar 14, 2024
@teodora-sandu teodora-sandu marked this pull request as ready for review March 14, 2024 17:27
@teodora-sandu teodora-sandu requested a review from a team as a code owner March 14, 2024 17:27
@@ -60,12 +69,15 @@ var retryErrorCodes = map[int]bool{
http.StatusInternalServerError: true,
}

// TODO: mutex lock for configuration?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The snyk-ls makes sure to not read this value if someone is writing to it, should we do this or leave it to snyk-ls to compute it when giving it to the client?

@teodora-sandu
Copy link
Collaborator Author

We think the license checks will continue to fail until this gets merged and I re-import the project.

Copy link

@chdorner-snyk chdorner-snyk left a comment

Choose a reason for hiding this comment

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

🍘

@teodora-sandu teodora-sandu merged commit cf02f1a into main Mar 15, 2024
8 checks passed
@teodora-sandu teodora-sandu deleted the refactor/snyk-code-client branch March 15, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants