Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Avoid panic in config finder #686

Merged
merged 1 commit into from
Apr 26, 2024
Merged

fix: Avoid panic in config finder #686

merged 1 commit into from
Apr 26, 2024

Conversation

anderseknert
Copy link
Member

Not sure if it's worth having two different exit conditions, but I guess it doesn't hurt with some granularity?

Signed-off-by: Anders Eknert <anders@styra.com>
@anderseknert anderseknert requested a review from srenatus April 26, 2024 08:59
Copy link
Member

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

I think I'd use more path/filepath here (like https://pkg.go.dev/path/filepath#Dir), but since a minimal change is good, LGTM.

@@ -139,6 +139,10 @@ func FindRegalDirectory(path string) (*os.File, error) {
return nil, errors.New("stopping as dir is empty string")
}

if len(parts) < 2 {
return nil, errors.New("stopping as dir is root directory")
}
Copy link
Member

Choose a reason for hiding this comment

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

Is "root directory" well defined for windows? How about C:\ or D:\ or \\?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, no idea. I hope this code isn’t permanent, as I’ll want to find out what’s going on here… but first to get a windows box somehow.

@anderseknert anderseknert merged commit e170834 into main Apr 26, 2024
3 checks passed
@anderseknert anderseknert deleted the avoid-panic-again branch April 26, 2024 09:58
srenatus pushed a commit to srenatus/regal that referenced this pull request Oct 1, 2024
Signed-off-by: Anders Eknert <anders@styra.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants