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

feat: support replace FieldParser #1043

Merged
merged 3 commits into from
Nov 8, 2021
Merged

Conversation

forsaken628
Copy link
Contributor

Describe the PR
Support replace FieldParser. Then we can use this as a libary and write custom feature.

Relation issue
no

Additional context
no

@codecov
Copy link

codecov bot commented Nov 8, 2021

Codecov Report

Merging #1043 (ce12e4d) into master (1780cd2) will increase coverage by 0.09%.
The diff coverage is 96.49%.

❗ Current head ce12e4d differs from pull request most recent head 2a69caa. Consider uploading reports for the commit 2a69caa to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1043      +/-   ##
==========================================
+ Coverage   92.96%   93.05%   +0.09%     
==========================================
  Files           7        8       +1     
  Lines        2018     2046      +28     
==========================================
+ Hits         1876     1904      +28     
  Misses         79       79              
  Partials       63       63              
Impacted Files Coverage Δ
parser.go 92.27% <70.00%> (-1.23%) ⬇️
field_parser.go 99.03% <99.03%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1780cd2...2a69caa. Read the comment docs.

@ubogdan
Copy link
Contributor

ubogdan commented Nov 8, 2021

@forsaken628 Looks good until now, congrats. We will need to have positive coverage on parser.go so we can merge it.

@forsaken628
Copy link
Contributor Author

I move a lot code to new file. So it's seem no possible have positive coverage on parser.go.

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 eff27cc into swaggo:master Nov 8, 2021
@HYY-yu
Copy link
Contributor

HYY-yu commented Nov 9, 2021

@forsaken628 The code in parser.go conflicts with my modifications, #1045 😄
But I think you did a great job refactoring the code, and I'll fix mine.

@@ -1053,13 +1057,25 @@ func (parser *Parser) parseStructField(file *ast.File, field *ast.Field) (map[st
return map[string]spec.Schema{typeName: *schema}, nil, nil
}

fieldName, schema, err := parser.getFieldName(field)
ps := newTagBaseFieldParser(parser, field)
Copy link
Contributor

Choose a reason for hiding this comment

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

@forsaken628 It should be:

ps := parser.fieldParserFactory(parser, field)

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