8000 Added compatibility with Latvian edoc format by vgaicuks · Pull Request #7 · thorgate/pyasice · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Added compatibility with Latvian edoc format #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Jul 28, 2021
Merged

Added compatibility with Latvian edoc format #7

merged 6 commits into from
Jul 28, 2021

Conversation

vgaicuks
Copy link
Contributor
@vgaicuks vgaicuks commented Jul 22, 2021 •
edited
Loading

ID attribute is required for SignatureTimeStamp tag.
Each of used certificate (CA root, OCSP and TS) must be added to CertificateValues tag.

What do you think about add_cert method which not so DRY on set_root_ca_cert point of view?

@vgaicuks vgaicuks marked this pull request as ready for review July 22, 2021 10:09
@Jyrno42
Copy link
Contributor
Jyrno42 commented Jul 26, 2021 •
edited
Loading

Hmm, maybe it is possible to unify these. For example, add a dictionary attrib argument to add_cert and then call it from set_root_ca_cert method too. This way everything would still be nicely DRY and your use-case would work too.

@Jyrno42 Jyrno42 self-requested a review July 26, 2021 08:03
Jyrno42
Jyrno42 requested changes Jul 27, 2021
View reviewed changes
Copy link
Contributor
@Jyrno42 Jyrno42 left a comment

Choose a reason for hiding this comment

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

see comment above

@vgaicuks
Copy link
Contributor Author

@Jyrno42, thank you for your response. I'll fix PR according to your advice.

vgaicuks
vgaicuks commented Jul 27, 2021
View reviewed changes
Copy link
Contributor Author
@vgaicuks vgaicuks left a comment

Choose a reason for hiding this comment

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

Could you please check my solution?

Jyrno42
Jyrno42 reviewed Jul 27, 2021
View reviewed changes
Jyrno42
Jyrno42 requested changes Jul 27, 2021
View reviewed changes
Copy link
Contributor
@Jyrno42 Jyrno42 left a comment

Choose a reason for hiding this comment

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

1 tiny thing

@Jyrno42
Copy link
Contributor
Jyrno42 commented Jul 28, 2021

Thanks for your contribution. I will try to get the updated version released tomorrow or Friday.

@Jyrno42 Jyrno42 merged commit f97ef53 into thorgate:main Jul 28, 2021
@vgaicuks
Copy link
Contributor Author

Guys, you are awesome, with pyasice project you saved aprox. one week of my time! It was a pleasure for me to cooperate with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorgenader jorgenader jorgenader left review comments

@Jyrno42 Jyrno42 Jyrno42 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vgaicuks @Jyrno42 @jorgenader
0