Skip to content

Conversation

Toilal
Copy link
Contributor

@Toilal Toilal commented Sep 14, 2017

This fix some issues that I think are mainly related to changes in dependencies and CFSSL api since this library was initialy written.

@codecov
Copy link

codecov bot commented Sep 14, 2017

Codecov Report

Merging #20 into master will decrease coverage by 5.71%.
The diff coverage is 27.27%.

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
- Coverage     100%   94.28%   -5.72%     
==========================================
  Files          13       13              
  Lines         131      140       +9     
  Branches        5        8       +3     
==========================================
+ Hits          131      132       +1     
- Misses          0        8       +8
Impacted Files Coverage Δ
cfssl/models/certificate_request.py 100% <100%> (ø) ⬆️
cfssl/cfssl.py 85.96% <20%> (-14.04%) ⬇️

@Toilal Toilal changed the title [FIX] Fix CertificateRequest default key configuration Various fixes and small enhancements Sep 14, 2017
@lasley lasley added this to the 0.1.0 milestone Sep 14, 2017
@lasley lasley added bug and removed enhancement labels Sep 14, 2017
Copy link
Member

@lasley lasley left a comment

Choose a reason for hiding this comment

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

Awesome thanks @Toilal!

Code looks 👍 , but Travis is failing.

@Toilal
Copy link
Contributor Author

Toilal commented Sep 14, 2017

I'll fix tests tomorrow

In some use case, it's easier to pass native structures (dict, string, ...) instead of domain objects (CertificateRequest, SubjectInfo, ...)
@codecov-io
Copy link

codecov-io commented Sep 20, 2017

Codecov Report

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

@@          Coverage Diff          @@
##           master    #20   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     14    +1     
  Lines         131    157   +26     
  Branches        5     12    +7     
=====================================
+ Hits          131    157   +26
Impacted Files Coverage Δ
cfssl/models/subject_info.py 100% <ø> (ø) ⬆️
cfssl/models/policy_sign.py 100% <100%> (ø) ⬆️
cfssl/cfssl.py 100% <100%> (ø) ⬆️
cfssl/models/config_mixer.py 100% <100%> (ø) ⬆️
cfssl/models/config_client.py 100% <100%> (ø) ⬆️
cfssl/models/certificate_request.py 100% <100%> (ø) ⬆️
cfssl/utils.py 100% <100%> (ø)

@Toilal
Copy link
Contributor Author

Toilal commented Sep 20, 2017

I've bring back 100% code coverage ! :)

I think it's ready to merge.

I've made another change today to allow raw API data structures instead of Domain objects (RequestCertificate, Host, SubjectInfo, and so one). They can now be replaced with string or dict, this will also work.

@Toilal
Copy link
Contributor Author

Toilal commented Sep 20, 2017

Maybe you could add Python 3 version in .travis.yml as it's now supported.

Could you perform a pipy release after merge ?

@lasley
Copy link
Member

lasley commented Sep 20, 2017

I've made another change today to allow raw API data structures instead of Domain objects (RequestCertificate, Host, SubjectInfo, and so one). They can now be replaced with string or dict, this will also work.

Good idea; I like.

Maybe you could add Python 3 version in .travis.yml as it's now supported.

I noticed your change in 2a4ed8a and created #21 to make the Travis change. Thanks for that!

Could you perform a pipy release after merge ?

Definitely

Copy link
Member

@lasley lasley left a comment

Choose a reason for hiding this comment

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

Thanks @Toilal!

@lasley lasley merged commit 6e7fa8e into LasLabs:master Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants