Skip to content

support MarshalText in custom type #2727

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

Merged
merged 4 commits into from
Feb 26, 2022

Conversation

emilgpa
Copy link
Contributor

@emilgpa emilgpa commented Feb 23, 2022

This PR gives supports MarshalText in custom type.
The problem is that possibly generates breaking change. So the solution here is:

  • Merge this PR in a major version.
  • Or to use a flag to enable MarshalText for custom type. Something like swagger generate spec -o ./swagger.json --marshalText

Personally, the second option can be good enough.

This code https://go.dev/play/p/tI99ntNYt62 generate this schema https://www.codepile.net/pile/KOGVJqym

I would like to create a couple of tests to verify that everything is correct but I don't have time to see how they are done in depth, could someone give me some clues where to start?

EDIT: I forgot: this generates the format of field to uuid if the custom type is named as uuid, UUID or Uuid (L259).

@emilgpa emilgpa changed the title support TextMarshal in Custom Type support TextMarshal in custom type Feb 23, 2022
@emilgpa emilgpa changed the title support TextMarshal in custom type support MarshalText in custom type Feb 23, 2022
@codecov
Copy link

codecov bot commented Feb 23, 2022

Codecov Report

Merging #2727 (fab7132) into master (5846010) will decrease coverage by 0.18%.
The diff coverage is 56.41%.

❗ Current head fab7132 differs from pull request most recent head c285f9b. Consider uploading reports for the commit c285f9b to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2727      +/-   ##
==========================================
- Coverage   82.91%   82.72%   -0.19%     
==========================================
  Files          55       55              
  Lines        9548     9583      +35     
==========================================
+ Hits         7917     7928      +11     
- Misses       1117     1133      +16     
- Partials      514      522       +8     
Impacted Files Coverage Δ
codescan/schema.go 67.43% <56.41%> (-1.81%) ⬇️

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 5846010...c285f9b. Read the comment docs.

@casualjim
Copy link
Member

Any chance you could make a test for this? I rarely touch this codebase and I don't want to break stuff in the future

@emilgpa
Copy link
Contributor Author

emilgpa commented Feb 26, 2022

@casualjim yes, I had planned to do it as I pointed out in the PR message. It is already added in the last commit seconds ago!

@casualjim casualjim merged commit b1a8dae into go-swagger:master Feb 26, 2022
@casualjim
Copy link
Member

Excellent, thanks

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.

2 participants