-
Notifications
You must be signed in to change notification settings - Fork 10
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
doc(swagger): add Swagger doc for all endpoints and reorganize routing #22
Conversation
Will review by this weekend. |
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.
Few comments, rest looks good to me.
cmd/laas/main.go
Outdated
"github.com/fossology/LicenseDb/pkg/api" | ||
"github.com/fossology/LicenseDb/pkg/db" | ||
"github.com/fossology/LicenseDb/pkg/models" | ||
"log" |
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 log should be above only. The order is std libraries and then third-party libs. You may use goimports -l -e -w pkg cmd test
to format and arrange the imports.
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.
Afterthought: we may use a tool like golangci-lint in our CI to automatically run these checks. I can add a PR if we agree.
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 formatting is currently done by my IDE. I think it will be a good idea if you create the CI workflow, I can update my IDE accordingly.
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.
Hope new formatting looks good.
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.
Yes, it is on point, I will add the static checks.
c8ad292
to
9f7bd70
Compare
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.
Looks great to me. Awesome work 👏🏼
9f7bd70
to
2d3e5e2
Compare
1. Add swagger documentation to all the endpoints. 2. Fix the status code of endpoints. 3. Make minor changes in DB schema. Signed-off-by: Gaurav Mishra <[email protected]>
1. Check if the latest swagger docs are commited with the PR. 2. Check if the swagger docs are formatted. Signed-off-by: Gaurav Mishra <[email protected]>
2d3e5e2
to
ddccd78
Compare
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.
LGTM
Note: Merge after #21