Skip to content

Conversation

Atheuz
Copy link
Contributor

@Atheuz Atheuz commented Apr 2, 2019

Change Summary

Added SecretStr and SecretBytes.

Related issue number

#443

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • HISTORY.rst has been updated

@Atheuz
Copy link
Contributor Author

Atheuz commented Apr 2, 2019

@samuelcolvin is this better?

@samuelcolvin
Copy link
Member

Looks like you've changed the permissions on lots of files 100644 → 100755. 😈

Maybe you could set back to 644?

@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #452 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #452   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          14     14           
  Lines        2188   2220   +32     
  Branches      435    435           
=====================================
+ Hits         2188   2220   +32

@Atheuz Atheuz force-pushed the add-secret-types2 branch from e485d56 to d7845a4 Compare April 2, 2019 16:21
@Atheuz
Copy link
Contributor Author

Atheuz commented Apr 2, 2019

@samuelcolvin fixed the issues you brought up, but didn't know about the format thing you mentioned.

@Atheuz Atheuz force-pushed the add-secret-types2 branch 2 times, most recently from af54f96 to fec9a1e Compare April 2, 2019 17:22
@Atheuz Atheuz force-pushed the add-secret-types2 branch from fec9a1e to 6372516 Compare April 2, 2019 17:32
@LasseGravesen
Copy link
Contributor

@samuelcolvin are we mergable yet?

@samuelcolvin
Copy link
Member

Just waiting for a response from @tiangolo, on the question above, I've done some research but not entirely clear.

Is this particularly urgent for you?

@tiangolo
Copy link
Contributor

tiangolo commented Apr 3, 2019

Sorry for the delay guys. I hadn't seen the notification. I added my comment in the conversation above.

@samuelcolvin
Copy link
Member

thanks @tiangolo, much appreciated.

No problem on the slight delay, still better than the response time for most paid services. :-)

Copy link
Contributor

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

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

Great! This looks very useful.

I added a couple of suggestions related to JSON Schema.

Copy link
Contributor

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

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

Just a couple of things I forgot to mention in docs.

Copy link
Contributor

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

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

Great! LGTM

@samuelcolvin samuelcolvin merged commit 4a8faca into pydantic:master Apr 4, 2019
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.

4 participants