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

feat: Enabling SCLE [NEBULA-1371] #447

Merged
merged 12 commits into from
Sep 11, 2023
Merged

feat: Enabling SCLE [NEBULA-1371] #447

merged 12 commits into from
Sep 11, 2023

Conversation

metju90
Copy link
Contributor

@metju90 metju90 commented Aug 28, 2023

Enables SCLE

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

detekt found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@metju90 metju90 marked this pull request as ready for review September 6, 2023 14:12
@metju90 metju90 requested a review from a team as a code owner September 6, 2023 14:12
@bastiandoetsch bastiandoetsch self-requested a review September 7, 2023 07:41
Copy link
Collaborator

@bastiandoetsch bastiandoetsch left a comment

Choose a reason for hiding this comment

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

Good job, just some small changes requested :)

@@ -29,11 +31,22 @@ class SnykApiClient(
log.debug("Executing request to $apiName")
val response = retrofitCall.execute()
if (!response.isSuccessful) {
if (response.code() == HttpStatusCode.UnprocessableEntity.value) {
val responseBodyString = response.errorBody()?.string()
val errorBody= Gson().fromJson<CliConfigSettingsError?>(responseBodyString, CliConfigSettingsError::class.java)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please fix the linter stuff :)

@@ -48,7 +48,9 @@ class SnykApplicationSettingsStateService : PersistentStateComponent<SnykApplica
var containerScanEnabled: Boolean = true

var sastOnServerEnabled: Boolean? = null
var sastSettingsError: Boolean? = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

linter :)

@@ -219,25 +221,34 @@ class ScanTypesPanel(
return
}

var sastSettingsError: String = ""
val sastCliConfigSettings: CliConfigSettings? =
try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

linter :)

Copy link
Collaborator

@bastiandoetsch bastiandoetsch left a comment

Choose a reason for hiding this comment

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

Nice! Only linting and a changelog entry are missing.

Signed-off-by: Matthew Barbara <[email protected]>
Comment on lines 3 to 17
import io.mockk.mockk
import io.mockk.every
import io.mockk.mockkStatic
import io.mockk.unmockkAll
import junit.framework.TestCase.assertFalse
import junit.framework.TestCase.assertTrue
import org.hamcrest.core.IsEqual.equalTo
import org.junit.Assert.assertThat
import org.junit.Test
import org.junit.Before
import java.net.URI
import io.snyk.plugin.pluginSettings
import io.snyk.plugin.services.SnykApplicationSettingsStateService
import junit.framework.TestCase.assertEquals
import org.junit.After

Check warning

Code scanning / detekt

Detects imports in non default order

Imports must be ordered in lexicographic order without any empty lines in-between with "java", "javax", "kotlin" and aliases in the end
Signed-off-by: Matthew Barbara <[email protected]>
Signed-off-by: Matthew Barbara <[email protected]>
@bastiandoetsch bastiandoetsch merged commit fa48159 into master Sep 11, 2023
@bastiandoetsch bastiandoetsch deleted the feat/enable-scle branch September 11, 2023 08:01
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