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

add functio scoped struct parse, tests #1283

Merged
merged 8 commits into from
Aug 16, 2022

Conversation

mstrYoda
Copy link
Contributor

@mstrYoda mstrYoda commented Aug 7, 2022

Describe the PR
I added the functionality of parsing function scoped structs.

With this development we will be able to define our request response types in the same function scope as our handlers:

package main

// @Param request body main.Fun.request true "query params" 
// @Success 200 {object} main.Fun.response
// @Router /test [post]
func Fun()  {
	type request struct {
		Name string
	}
	
	type response struct {
		Name string
		Child string
	}
}

Relation issue
#1274

Additional context
All tests passed in parser_tests.go. Changes look backward compatible.

@mstrYoda
Copy link
Contributor Author

mstrYoda commented Aug 9, 2022

@ubogdan Hi, can you review the changes when you are available? 🙏

@codecov
Copy link

codecov bot commented Aug 9, 2022

Codecov Report

Merging #1283 (47a1848) into master (2f148dd) will increase coverage by 0.09%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
+ Coverage   94.95%   95.04%   +0.09%     
==========================================
  Files          14       14              
  Lines        2717     2787      +70     
==========================================
+ Hits         2580     2649      +69     
- Misses         78       79       +1     
  Partials       59       59              
Impacted Files Coverage Δ
parser.go 93.89% <88.88%> (+0.17%) ⬆️
packages.go 90.66% <94.44%> (+0.71%) ⬆️
schema.go 100.00% <100.00%> (ø)
types.go 100.00% <100.00%> (ø)
generics.go 94.61% <0.00%> (+0.30%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ubogdan
ubogdan previously approved these changes Aug 9, 2022
Copy link
Contributor

@ubogdan ubogdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ubogdan
Copy link
Contributor

ubogdan commented Aug 9, 2022

Please improve the unit test for newly added code.

@mstrYoda
Copy link
Contributor Author

mstrYoda commented Aug 9, 2022

Please improve the unit test for newly added code.

I improved the unit tests according to changes 🙏

@ubogdan
Copy link
Contributor

ubogdan commented Aug 10, 2022

Build failed

@mstrYoda
Copy link
Contributor Author

Build failed

Actually make test is passing on my local. Can we retry the pipeline?

@ubogdan
Copy link
Contributor

ubogdan commented Aug 11, 2022

The error is obvious https://github.com/swaggo/swag/runs/7767202306?check_suite_focus=true , Try to run the tests against a go version lower than 1.18.

@mstrYoda mstrYoda requested a review from ubogdan August 12, 2022 06:53
@ubogdan
Copy link
Contributor

ubogdan commented Aug 12, 2022

@mstrYoda
Copy link
Contributor Author

Hey @ubogdan, I need your opinion here:

  • Do we need to add the newly created method parseFunctionScopedTypesFromFile() to packages.loadExternalPackage() L260 too?

@mstrYoda
Copy link
Contributor Author

Hey @ubogdan, I need your opinion here:

  • Do we need to add the newly created method parseFunctionScopedTypesFromFile() to packages.loadExternalPackage() L260 too?

Hi @ubogdan what about it 🤔 Can we able to merge it as is?

@ubogdan
Copy link
Contributor

ubogdan commented Aug 16, 2022

@mstrYoda I'm quite busy. I'm glad you figured out by yourself.

Copy link
Contributor

@ubogdan ubogdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ubogdan ubogdan merged commit 45f01a1 into swaggo:master Aug 16, 2022
@ubogdan
Copy link
Contributor

ubogdan commented Aug 16, 2022

@mstrYoda Thanks for your contribution!

@mstrYoda
Copy link
Contributor Author

@mstrYoda Thanks for your contribution!

Thank you for your effort 🙏 sorry for the mentioning you a lot :)

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.

None yet

2 participants