search

runfinch / finch-daemon

Apache License 2.0
7 stars 10 forks source link

Refactor project structure #16

Closed henry118 closed 2 months ago

henry118 commented 3 months ago

Refactor project structure:

  1. Move the following packages out of pkg as they are not meant for export
    • /pkg/api -> /api
    • /pkg/backend -> /internal/backend
    • /pkg/service -> /internal/service
    • /pkg/mocks -> /mocks
  2. Refactor /internal/backend and split different services into separate files

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

pendo324 commented 3 months ago

LGTM.

As part of review, I ran the unit and e2e tests from this PR's branch and noticed that this test is failing:

• [FAILED] [0.946 seconds]
Finch Daemon Functional test version API [It] should successfully get the version info
/Users/alvajus/Code/finch-daemon/e2e/tests/version.go:29

  Timeline >>
  2c38bb63ac831cd49e0eaaf3bde2355ac1fd2cd265c6c615a1fcd9c46bd5b810
  No containers to be removed
  12120425f07d
  No images to be removed
  f29b1f8aa29d11fa49ca6eb70342eeb95d08cdc651d312141d80ebb1d0b9a9ce
  bridge
  host
  none
  No networks to be removed
  [FAILED] in [It] - /Users/alvajus/Code/finch-daemon/e2e/tests/version.go:38 @ 08/26/24 18:12:41.738
  << Timeline

  [FAILED] Expected
      <string>:
  not to be empty
  In [It] at: /Users/alvajus/Code/finch-daemon/e2e/tests/version.go:38 @ 08/26/24 18:12:41.738

I don't think its related to this change, but it would be good to chase this down and fix it. I can create a new issue for tracking if you want.

henry118 commented 3 months ago

As part of review, I ran the unit and e2e tests from this PR's branch and noticed that this test is failing:

I'll take a look

henry118 commented 3 months ago

As part of review, I ran the unit and e2e tests from this PR's branch and noticed that this test is failing:

I'll take a look

OK so it's indeed because of this change. I forgot to update version package path we use in the Makefile to patch the version number. This is fixed in the updated revision.

Additionally I don't think finch-daemon needs MacOS for its e2e tests though. I'll open another PR to change that.