Skip to content

Conversation

@tevoinea
Copy link
Member

@tevoinea tevoinea commented Dec 6, 2021

Using go cli to scan is more accurate than parsing go.sum files, but go projects aren't always restored in a workflow. This results in go restoring packages in a workflow where it's not required. By allowing an opt-in we give users the power to make the right choice for their specific situations.

We still fall back to manual parsing if go cli fails.

@tevoinea tevoinea requested a review from a team as a code owner December 6, 2021 16:21
@tevoinea tevoinea requested a review from chsalgado December 6, 2021 16:21
@github-actions
Copy link

github-actions bot commented Dec 6, 2021

👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:

  • The detector detects more or fewer components than before
  • The detector generates different parent/child graph relationships than before
  • The detector generates different devDependencies values than before

If none of the above scenarios apply, feel free to ignore this comment 🙂

@tevoinea tevoinea linked an issue Dec 9, 2021 that may be closed by this pull request
"preLaunchTask": "build",
// If you have changed target frameworks, make sure to update the program path.
"program": "${workspaceFolder}/src/Microsoft.ComponentDetection/bin/Debug/net6.0/Microsoft.ComponentDetection.dll",
"program": "${workspaceFolder}/src/Microsoft.ComponentDetection/bin/Debug/netcoreapp3.1/Microsoft.ComponentDetection.dll",
Copy link
Member Author

Choose a reason for hiding this comment

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

Tiny thing we missed moving back to netcore 3.1

Comment on lines +61 to +64
catch
{
Logger.LogInfo("Failed to detect components using go cli.");
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you log the exception here


private bool IsGoCliManuallyEnabled()
{
return EnvVarService.DoesEnvironmentVariableExist("EnableGoCliScan");
Copy link
Member

Choose a reason for hiding this comment

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

Can you create a constants class for environment variables? If we're going to create a lot of these, it'll be easier to keep track of.

Copy link
Member

@JamieMagee JamieMagee left a comment

Choose a reason for hiding this comment

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

Couple of small things, but nothing blocking.

@tevoinea tevoinea merged commit 70c149c into main Dec 17, 2021
@tevoinea tevoinea deleted the tevoinea/EnableGoCliOptIn branch December 17, 2021 17:19
@JamieMagee JamieMagee added type:ci Build or repository configuration and removed repo-configuration labels Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:ci Build or repository configuration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable opt in to go cli scanning

3 participants