Skip to content

Conversation

@lukasgee
Copy link

Fixed incorrect default value for the tags of type Set from the list [] to the actual set set() according to initial type.

@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #2646 (73deb11) into master (f031973) will not change coverage.
The diff coverage is n/a.

❗ Current head 73deb11 differs from pull request most recent head 36b063e. Consider uploading reports for the commit 36b063e to get more accurate results

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2646   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          243       243           
  Lines         7419      7419           
=========================================
  Hits          7419      7419           

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 cb40e27...36b063e. Read the comment docs.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 73deb11 at: https://5fff5d61b3aa71452da3f60b--fastapi.netlify.app

@lukasgee lukasgee changed the title Fixed incorrect defaul value for type Set Fixed incorrect default value for type Set Jan 13, 2021
Copy link
Contributor

@ycd ycd left a comment

Choose a reason for hiding this comment

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

Nice catch!

This was referenced Feb 11, 2021
Copy link
Contributor

@cikay cikay left a comment

Choose a reason for hiding this comment

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

The commit message subject should be "Make the default data type of tags value set"
You can also explain the reason in the body of the commit message

@lukasgee lukasgee force-pushed the fix-body-nested-models-item-default branch from 73deb11 to 36b063e Compare February 6, 2022 20:03
@lukasgee
Copy link
Author

lukasgee commented Feb 6, 2022

@cikay Changed it as you requested. Hope it's fine now. Thanks for the hint!

@tiangolo
Copy link
Member

Thanks for the effort @lukasgee! 🍪

And thanks for the help @ycd! ☕

Thanks for the interest @cikay, but the commit message is not too important, when I merge I use squash, and I update the PR title before that to keep it consistent with the other commits.

This was covered in a recent PR, so I'm gonna close this one now.

@tiangolo tiangolo closed this Aug 22, 2022
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.

5 participants