-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: parse only specific extension tag #1219
Conversation
This is not 100% ready yet. Let us get back |
Codecov ReportBase: 85.16% // Head: 85.22% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1219 +/- ##
==========================================
+ Coverage 85.16% 85.22% +0.06%
==========================================
Files 19 19
Lines 3620 3635 +15
==========================================
+ Hits 3083 3098 +15
Misses 459 459
Partials 78 78
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This is a nice and must-have feature. 👍 |
@igorkova let me know when you are done with this. |
2be97c4
to
36e1dc5
Compare
@ubogdan This is done as far as we are concerned. Example usage will be |
d1dcbbb
to
143e0bb
Compare
@ubogdan great if we can get workflow approved 🙌 |
Previous implementation had an issue where it was excluding operation after schema and other parts were parsed. That caused schema definition to be included in the output file, even though operation didn't match the extension. New implementation won't even start processing operation if extension isn't matching. Only potential issue/problem I see with this approach is that we are duplicating logic for comment line parsing (I basically c/p it from other places) but not sure how big of an issue that actually is as I noticed we are doing that at other places as well.
} | ||
return true | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing you are using from the parser is parser.ParseExtension, This function is a mimic of in_array func in_array(val string, array []string) bool
with default to true if the search value is empty.
Such a simple function should not be attached to the parser.
func matchExtension(textToFind string, comments []*ast.Comment) bool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
I was basically c/p above matchTags without too much thinking. you have a good point.
I can detach it from the parser, before that just to double check do you recommend to create a "helper" in_array-type method or still keep it specific to extension parsing? (Not sure if we have such a helper file/methods today somewhere - can't find them in the repo)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just detach the function from the parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Great job till now. |
@igorkova Thanks for your contribution. |
Describe the PR
Enable parsing (output) of specific operations which have specific tag (extension).
Relation issue
#661
Additional context