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

Fix bugs 2 #116

Merged
merged 2 commits into from
Nov 20, 2024
Merged

Fix bugs 2 #116

merged 2 commits into from
Nov 20, 2024

Conversation

victornicolet
Copy link
Contributor

This PR fixes bugs in the code:

  • possible nil-panics identified with Nilaway. Adds Nilaway as a check on the argot executable.
  • management of reports directory and "project-root": now the project root is computed as an absolute path and all other paths are taken relatively to this.

@victornicolet victornicolet requested review from samarth-aws and amzn-jasonrk and removed request for samarth-aws November 20, 2024 17:36
// Get absolute path to config. Other files in config will be relative to where the config is.
pathToConfig := filename
if !path.IsAbs(filename) {
wd, _ := os.Getwd()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably return the error

@@ -226,7 +227,8 @@ func TestLoadVersionBefore_v0_3_0_Errors(t *testing.T) {

func TestLoadWithReports(t *testing.T) {
c := NewDefault()
c.ReportsDir = "example-report"
wd, _ := os.Getwd()
c.ReportsDir = path.Join(wd, "testdata/example-report")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think filepath.Join is preferred over path.Join for better OS compatibility: https://pkg.go.dev/path/filepath#Join

// If it's a named target, need to change to project root's directory to properly load the target
err := os.Chdir(cfg.Root())
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: missing error context

@victornicolet victornicolet merged commit c686c66 into mainline Nov 20, 2024
3 checks passed
@victornicolet victornicolet deleted the fix-bugs-2 branch November 20, 2024 20:14
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