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

move request_builder into internal pkg (#304) #329

Conversation

vvatanabe
Copy link
Collaborator

@vvatanabe vvatanabe commented May 28, 2023

Fix #304 (request_builder)
I moved the following files to the internal package.

request_builder.go -> internal/request_builder.go
request_builder_test.go -> internal/request_builder_test.go

Internal/request_builder_test.go was circularly referenced. So I moved the following test to client_test.go. They are tests for error cases resulting from calls to methods of openai.Client. So I decided it was appropriate to move them to client_test.go.

request_builder_test.go:TestClientReturnsRequestBuilderErrors -> client_test.go:TestClientReturnsRequestBuilderErrors
request_builder_test.go:TestReturnsRequestBuilderErrorsAddtion -> client_test.go:TestClientReturnsRequestBuilderErrorsAddtion

@codecov
Copy link

codecov bot commented May 28, 2023

Codecov Report

Merging #329 (a6d1c8f) into master (62eb4be) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #329      +/-   ##
==========================================
+ Coverage   92.36%   92.45%   +0.08%     
==========================================
  Files          19       18       -1     
  Lines         655      636      -19     
==========================================
- Hits          605      588      -17     
+ Misses         37       35       -2     
  Partials       13       13              
Impacted Files Coverage Δ
chat.go 100.00% <100.00%> (ø)
client.go 91.96% <100.00%> (ø)
completion.go 100.00% <100.00%> (ø)
edits.go 100.00% <100.00%> (ø)
embeddings.go 100.00% <100.00%> (ø)
engines.go 100.00% <100.00%> (+13.33%) ⬆️
files.go 82.35% <100.00%> (ø)
fine_tunes.go 100.00% <100.00%> (ø)
image.go 92.77% <100.00%> (ø)
models.go 100.00% <100.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

@vvatanabe vvatanabe changed the title [WIP] move request_builder into internal pkg (#304) move request_builder into internal pkg (#304) May 28, 2023
@vvatanabe
Copy link
Collaborator Author

@sashabaranov Please check it.

@sashabaranov
Copy link
Owner

@vvatanabe thank you for the PR! Could you please update the tests so the coverage would not go down?

@vvatanabe vvatanabe force-pushed the move-request_builder-into-internal-pkg-304 branch from 8e9c035 to ef47d31 Compare May 30, 2023 00:55
@vvatanabe vvatanabe force-pushed the move-request_builder-into-internal-pkg-304 branch from ef47d31 to a6d1c8f Compare May 30, 2023 03:55
@vvatanabe
Copy link
Collaborator Author

@sashabaranov The following tests were added to increase coverage.

  • internal/request_builder_test.go
  • engines_test.go

FYI: We can view the codes not covered by the following commands in browser. It was very easy to detect.

$ go test -coverprofile=cover.out . /...
$ go tool cover -html=cover.out -o cover.html
$ open cover.html

@sashabaranov sashabaranov merged commit 61ba5f3 into sashabaranov:master May 31, 2023
3 checks passed
@vvatanabe vvatanabe deleted the move-request_builder-into-internal-pkg-304 branch May 31, 2023 08:22
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.

Move some code into internal directory
2 participants