Skip to content

Conversation

ybressler
Copy link
Contributor

@ybressler ybressler commented Jul 5, 2023

Change Summary

Small enhancements to migration documentation – explicitly call out the breaking change of optional fields. (Related to #6463)

Note:
I created 2 alternative tables for demonstrating how fields are defined, cause I'm not sure which one we should go with. (Or, if we should keep it as a bullet pointed list. Or, if we should scrap it and just keep the code example below it.)

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @lig

@ybressler
Copy link
Contributor Author

please review

Copy link
Contributor

@dmontagu dmontagu left a comment

Choose a reason for hiding this comment

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

One other thing that I think would be good — in V1, all of this also applied for fields annotated as Any. It might be worth adding a note that this has changed as well — fields annotated as Any no longer have a default of None either.

ybressler and others added 4 commits July 5, 2023 19:23
Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>
Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>
Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>
@ybressler ybressler requested a review from dmontagu July 5, 2023 23:32
@pydantic-hooky pydantic-hooky bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels Jul 6, 2023
@pydantic-hooky pydantic-hooky bot assigned ybressler and unassigned lig Jul 6, 2023
ybressler and others added 3 commits July 5, 2023 21:47
Co-authored-by: Serge Matveenko <lig@countzero.co>
Co-authored-by: Serge Matveenko <lig@countzero.co>
@ybressler ybressler requested a review from lig July 6, 2023 01:50
Copy link
Contributor

@lig lig left a comment

Choose a reason for hiding this comment

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

👍🏻 looks top-notch

@tpdorsey tpdorsey merged commit 8466f78 into pydantic:main Jul 6, 2023
@ybressler ybressler deleted the docs/enhancement_to_migration_guide branch July 6, 2023 14:34
@dimbleby
Copy link

dimbleby commented Jul 6, 2023

the second of these entries seems just to be a less-fully-explained copy of the first?

| Not required, cannot be `None`, is `'abc'` by default | `f3: str = 'abc'`           |
| Not required, cannot be `None`                        | `f4: str = 'Foobar'`        |

@lig
Copy link
Contributor

lig commented Jul 6, 2023

@dimbleby Nice catch. Thanks! Would you like making a PR?;)

@dimbleby
Copy link

dimbleby commented Jul 6, 2023

negative, sorry!

@tpdorsey
Copy link
Contributor

tpdorsey commented Jul 6, 2023

I will follow up on this. Thanks for pointing it out @dimbleby Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting author revision awaiting changes from the PR author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants