-
Notifications
You must be signed in to change notification settings - Fork 24
fix: Fail on empty tables #796
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
Conversation
6999422 to
e93ae27
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #796 +/- ##
==========================================
+ Coverage 46.63% 46.65% +0.02%
==========================================
Files 76 76
Lines 7820 7823 +3
==========================================
+ Hits 3647 3650 +3
Misses 3682 3682
Partials 491 491
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. β View full report in Codecov by Sentry. |
β±οΈ Benchmark results
|
yevgenypats
left a comment
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.
small but important nit imo :)
specs/source.go
Outdated
| } | ||
|
|
||
| if len(s.Tables) == 0 { | ||
| return fmt.Errorf("tables configuration is required. Hint: try setting tables to [\"*\"] to sync all tables or use `cloudquery tables` to list available tables") |
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.
I think we shouldn't recommend * because then we are back to square 1 in terms of usability. Hint: choose the tables you want to sync
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.
π― Wasn't sure about that hint too. See 6ef586c
π€ I have created a release *beep* *boop* --- ## [2.3.8](v2.3.7...v2.3.8) (2023-04-20) ### Bug Fixes * Fail on empty tables ([#796](#796)) ([1320d32](1320d32)) * **testing:** Add sorting for testing dest migrations ([#814](#814)) ([b1437f1](b1437f1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
Follow up to #790
Use the following steps to ensure your PR is ready to be reviewed
go fmtto format your code πgolangci-lint runπ¨ (install golangci-lint here)