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

chore!: eliminate dependency on the gaf #14

Merged
merged 2 commits into from
Mar 25, 2024
Merged

chore!: eliminate dependency on the gaf #14

merged 2 commits into from
Mar 25, 2024

Conversation

cmars
Copy link
Contributor

@cmars cmars commented Mar 22, 2024

The go-application-framework should not be a required dependency of this package. It provides only minimal supporting functionality:

  • Convenience of access to a logger
  • Convenience of two configuration options (Organization and IsFedramp)

while pulling in a lot of dependencies that a Snyk Code API client does not really need. It should not be necessary, for example, to have to know about the GAF's workflow engine in order to work with Snyk Code.

This change removes the dependency on the GAF, while leaving room to integrate the code client with the GAF in the CLI and LS. This reduction of coupling makes the open-source Snyk Code client more relevant to a much wider context of reuse.

@cmars cmars requested a review from a team as a code owner March 22, 2024 17:06
@CLAassistant
Copy link

CLAassistant commented Mar 22, 2024

CLA assistant check
All committers have signed the CLA.

@@ -264,8 +259,6 @@ func TestSnykCodeBackendServicePact_LocalCodeEngine(t *testing.T) {
Headers: dsl.MapMatcher{
"Content-Type": dsl.String("application/json"),
"snyk-request-id": getSnykRequestIdMatcher(),
"Session-Token": dsl.Regex("token fc763eba-0905-41c5-a27f-3934ab26786c", sessionTokenMatcher),
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes happened because we stopped setting up the configuration with the authentication token, which behind-the-scenes is used to configure these headers for the local code engine mode. I wanted to keep them so that we can make sure the local code engine mode continues to work during this refactoring but I think I can add a test in snyk/snyk-ls#455 instead to make sure these headers are set

http/http.go Outdated Show resolved Hide resolved
@teodora-sandu
Copy link
Contributor

The security check from jennifer.gorski was because of me - I had imported the repo there to verify something. I've removed it now and if we close and re-open the PR it will disappear but we can just merge this anyway.

The go-application-framework should not be a required dependency of this
package. It provides only minimal supporting functionality:

- Convenience of access to a logger
- Convenience of two configuration options (Organization and IsFedramp)

While pulling in a lot of dependencies that a Snyk Code API client does not
really need. It should not be necessary, for example, to have to know about the GAF's
workflow engine in order to work with Snyk Code.

This change removes the dependency on the GAF, while leaving room to integrate
the code client with the GAF in the CLI and LS. This reduction of coupling
makes the open-source Snyk Code client more relevant to a much wider context of
reuse.
@cmars cmars force-pushed the chore/eliminate-gaf branch 2 times, most recently from a56cbc4 to 52f12e8 Compare March 25, 2024 14:31
@cmars cmars merged commit 7412300 into main Mar 25, 2024
10 checks passed
@cmars cmars deleted the chore/eliminate-gaf branch March 25, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants