Skip to content
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

Re-organize project #88

Merged
merged 5 commits into from
May 11, 2021
Merged

Re-organize project #88

merged 5 commits into from
May 11, 2021

Conversation

dave-tucker
Copy link
Collaborator

  • move all ovsdb notation to ovsdb. this provides a clean separation between
    a) marshalling/unmarshalling of ovsdb datatypes and mapping to go types
    b) client code (e.g cache, orm, api etc...)
  • examples/stress and examples/print_schema to cmd as they are useful commands (more than examples)
  • fixes the error messages to be lowercase (which gopls insists on)
  • adds a .golangci.yml to attempt to replicate ^ but doesn't quite yet - i need to investigate this more thoroughly

This moves all ovsdb notation to it's own package
As such, everything ovsdb related (including changing
ovsdb types to go types) resides in `ovsdb`.
ALl client-specific code remains in the `libovsdb` package

Signed-off-by: Dave Tucker <[email protected]>
@dave-tucker dave-tucker requested a review from amorenoz May 7, 2021 17:17
@coveralls
Copy link

coveralls commented May 7, 2021

Pull Request Test Coverage Report for Build 831148708

  • 90 of 113 (79.65%) changed or added relevant lines in 12 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-3.9%) to 74.613%

Changes Missing Coverage Covered Lines Changed/Added Lines %
client/api.go 11 12 91.67%
ovsdb/schema.go 0 1 0.0%
client/model.go 5 7 71.43%
client/cache.go 4 7 57.14%
client/client.go 24 27 88.89%
ovsdb/bindings.go 23 26 88.46%
client/orm.go 13 18 72.22%
client/orm_info.go 6 11 54.55%
Totals Coverage Status
Change from base Build 817648290: -3.9%
Covered Lines: 1349
Relevant Lines: 1808

💛 - Coveralls

@amorenoz
Copy link
Collaborator

amorenoz commented May 10, 2021

I haven't taken a deep look but sharing a couple of initial thoughts:

  • Should we put subpackates into "pkg" directory to comply with the 'standard' golang project layout?
  • Should we put the client code into another subpackage so that the user has a more clear idea of what the packages do?
    E.g:
import "github.com/go-ovn/libovsdb/pkg/client"
import "github.com/go-ovn/libovsdb/pkg/ovsdb"

vs

import "github.com/go-ovn/libovsdb"
import "github.com/go-ovn/libovsdb/pkg/ovsdb"

They are more than examples, and are actually useful
utilities. As such, they belong in cmd

Signed-off-by: Dave Tucker <[email protected]>
go-staticcheck likes all error messages to be lowercase

Signed-off-by: Dave Tucker <[email protected]>
@dave-tucker
Copy link
Collaborator Author

@amorenoz based on our chat yesterday I've updated this.
We have a dedicated client package as well as an ovsdb package.
These don't live in the pkg directory, as that is typically used for useful packages that could be used by others, independent from the primary use of the library.

@amorenoz
Copy link
Collaborator

@dave-tucker LGTM, only comment is, if we ever want to have another module (e.g: server) use the ORM layer, it would have to be moved to a common place. It's all private so it would not affect the client API I believe

@dave-tucker
Copy link
Collaborator Author

@amorenoz good point. I'll merge this as-is, and we can look at splitting out the orm layer in to it's own package later.

@dave-tucker dave-tucker merged commit 09a5a1e into ovn-org:main May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants