-
Notifications
You must be signed in to change notification settings - Fork 1
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
FS-1101; Fixing fleetdb mod name #14
Conversation
…fleet-scheduler. This will break some stuff. . . Also adding a small script to make it easier to install crdb
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
=======================================
Coverage 72.72% 72.72%
=======================================
Files 38 38
Lines 3732 3732
=======================================
Hits 2714 2714
Misses 760 760
Partials 258 258
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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 tried to mark every place I saw a serverservice
or a hollow
. We should get rid of all that junk in this pass as well.
I like the "install crdb" script but I wonder if it might be better to just document it?
pkg/api/v1/mock_client_test.go
Outdated
@@ -6,7 +6,7 @@ import ( | |||
"io" | |||
"net/http" | |||
|
|||
serverservice "go.hollow.sh/serverservice/pkg/api/v1" | |||
serverservice "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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.
it's only a mock but this should be fleetdb
pkg/api/v1/router_bom_test.go
Outdated
@@ -6,7 +6,7 @@ import ( | |||
"strings" | |||
"testing" | |||
|
|||
serverservice "go.hollow.sh/serverservice/pkg/api/v1" | |||
serverservice "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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.
s/serverservice/fleetdb
pkg/api/v1/router_firmware_test.go
Outdated
"go.hollow.sh/serverservice/internal/dbtools" | ||
serverservice "go.hollow.sh/serverservice/pkg/api/v1" | ||
"github.com/metal-toolbox/fleetdb/internal/dbtools" | ||
serverservice "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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 will keep noting it too. 😅
pkg/api/v1/router_int_test.go
Outdated
hollow "go.hollow.sh/serverservice/pkg/api/v1" | ||
"github.com/metal-toolbox/fleetdb/internal/dbtools" | ||
"github.com/metal-toolbox/fleetdb/internal/httpsrv" | ||
hollow "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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.
hollow
doesn't have any meaning for us. fleetdb
or fleetdbApi
or whatever.
"go.hollow.sh/serverservice/internal/dbtools" | ||
serverservice "go.hollow.sh/serverservice/pkg/api/v1" | ||
"github.com/metal-toolbox/fleetdb/internal/dbtools" | ||
serverservice "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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.
Token replacement
pkg/api/v1/router_server_test.go
Outdated
"go.hollow.sh/serverservice/internal/dbtools" | ||
serverservice "go.hollow.sh/serverservice/pkg/api/v1" | ||
"github.com/metal-toolbox/fleetdb/internal/dbtools" | ||
serverservice "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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.
token replacement
@@ -8,7 +8,7 @@ import ( | |||
"github.com/stretchr/testify/assert" | |||
"github.com/stretchr/testify/require" | |||
|
|||
hollow "go.hollow.sh/serverservice/pkg/api/v1" | |||
hollow "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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.
change hollow
to something else please
pkg/api/v1/server_service_test.go
Outdated
@@ -9,7 +9,7 @@ import ( | |||
"github.com/stretchr/testify/assert" | |||
"github.com/stretchr/testify/require" | |||
|
|||
hollow "go.hollow.sh/serverservice/pkg/api/v1" | |||
hollow "github.com/metal-toolbox/fleetdb/pkg/api/v1" |
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 wonder if hollow
means anything to anyone still here.
scripts/install_crdb.sh
Outdated
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 like the idea, but I wonder if there isn't an easier way via a package manager? I used brew install cockroach
.
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 was simply following the instructions in the README. I keep forgetting homebrew exists. I just checked and apt and snap doesnt have it. I can rename this and remove the macOS side of it.
Once this is in, this can be merged. |
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.
A couple of mis-types?
README.md
Outdated
@@ -1,9 +1,9 @@ | |||
# Server Service | |||
# FleetDB | |||
|
|||
> This repository is experimental meaning that it's based on untested ideas or techniques and not yet established or finalized or involves a radically new and innovative style! | |||
> This means that support is best effort (at best!) and we strongly encourage you to NOT use this in production. |
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 only just saw that we still have this "It's untested and untrusted" thing here. We should ditch that, but if you don't want to in they PR add a Jira ticket.
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.
Done
Changing go mod name from
to
Bonus: