-
Notifications
You must be signed in to change notification settings - Fork 34
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
Return the error that ReadDir returns in localSchemasClient.Paths #78
Return the error that ReadDir returns in localSchemasClient.Paths #78
Conversation
Welcome @hiroyaonoe! |
@@ -32,7 +32,10 @@ func (k *localSchemasClient) Paths() (map[string]openapi.GroupVersion, error) { | |||
return nil, nil | |||
} | |||
res := map[string]openapi.GroupVersion{} | |||
apiGroups, _ := utils.ReadDir(k.fs, filepath.Join(k.dir, "apis")) | |||
apiGroups, err := utils.ReadDir(k.fs, filepath.Join(k.dir, "apis")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The apis
folder is allowed to not exist, we should return an error only if the folder exists AND there was an error reading it.
@@ -48,7 +51,10 @@ func (k *localSchemasClient) Paths() (map[string]openapi.GroupVersion, error) { | |||
res[path] = groupversion.NewForFile(k.fs, filepath.Join(groupPath, v.Name())) | |||
} | |||
} | |||
coregroup, _ := utils.ReadDir(k.fs, filepath.Join(k.dir, "api")) | |||
coregroup, err := utils.ReadDir(k.fs, filepath.Join(k.dir, "api")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The api
folder is allowed to not exist, we should return an error only if the folder exists AND there was an error reading it.
c010dab
to
5b3a4a6
Compare
Thanks for your review. |
Thanks @hiroyaonoe can you add unit tests ? |
5b3a4a6
to
4cf7a50
Compare
I added a unit test for the case where it is not a directory. |
Thanks @hiroyaonoe and @eddycharly ! /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexzielenski, hiroyaonoe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #75
If ReadDir returns an error except for fs.ErrNotExist, localSchemasClient.Paths returns the error with an additional message.