search

pydantic / pydantic-extra-types

Extra Pydantic types.
MIT License
181 stars 48 forks source link

:sparkles: adding new type iban #65

Closed kroncatti closed 8 months ago

kroncatti commented 1 year ago

Resolves #10

Selected Reviewer: @yezz123

yezz123 commented 1 year ago

please review

kroncatti commented 1 year ago

We have more properties that could be added as shown in here. Do you think it's worth it ?

yezz123 commented 1 year ago

We have more properties that could be added as shown in here. Do you think it's worth it ?

Yes its totally worth it 👍🏻

kroncatti commented 1 year ago

I'll do it still on this PR then, thanks. Possibly today.

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (5216db3) 100.00% compared to head (ed611a0) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #65 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 10 11 +1 Lines 665 726 +61 Branches 166 183 +17 ========================================= + Hits 665 726 +61 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kroncatti commented 1 year ago

Just did @yezz123, sorry for the time it took. If I did not miss anything, we should be good.

kroncatti commented 1 year ago

@yezz123 now we're ready for review 😄

kroncatti commented 1 year ago

Resolves #10

samuelcolvin commented 1 year ago

Resolves #10

please put this in the PR body so it closes the issue upon merge.

kroncatti commented 1 year ago

Done @samuelcolvin, thanks.

yezz123 commented 1 year ago

sorry for late review @kroncatti i will review it again today and test it then we can merge it 🙏🏻

Kludex commented 1 year ago

Please don't merge this yet.

Let's try to get __get_pydantic_core_schema__ merged into https://github.com/mdomke/schwifty/ first.

lig commented 1 year ago

@Kludex I've just request a review from you on this. You have control now over when it could be merged.

Kludex commented 1 year ago

@Kludex I've just request a review from you on this. You have control now over when it could be merged.

So much power! 👀

I've opened https://github.com/mdomke/schwifty/issues/147 on schwifty. 🙏

kroncatti commented 1 year ago

Amazing folks! thanks!

Kludex commented 1 year ago

Amazing folks! thanks!

dnd :)

kroncatti commented 1 year ago

Hey folks,

It's been a few weeks we are waiting on this. What is you guys opinion ? Should we wait to merge ?

Kludex commented 1 year ago

Hey folks,

It's been a few weeks we are waiting on this. What is you guys opinion ? Should we wait to merge ?

I think we should wait for https://github.com/mdomke/schwifty/issues/147.

waveman68 commented 1 year ago

Hey folks,

It's been a few weeks we are waiting on this. What is you guys opinion ? Should we wait to merge ?

I vote to move forward.

yezz123 commented 8 months ago

This PR is ready once we drop support for Python 3.7 🙏🏻 .

cc @kroncatti @Kludex

mdomke commented 8 months ago

@kroncatti @yezz123 @Kludex Hi! I've implemented the Pydantic protocol support in https://github.com/mdomke/schwifty/issues/147 last year November. Sorry that it took me a while, but I was quite busy at the time. Is the implementation in schwifty working for you or is there something that you need additionally, so that you have to create this wrapper type?

Kludex commented 8 months ago

Oh, nice! Maybe we can add that to the Pydantic documentation?