Skip to content

Conversation

@npmccallum
Copy link
Contributor

This PR derives implementations for x509 types. I have taken great care to ensure that every commit passes all existing tests. This PR is the early portion of the larger #414 PR.

@npmccallum npmccallum mentioned this pull request Feb 11, 2022
5 tasks
@tarcieri
Copy link
Member

@carl-wallace mind taking a look at this one?

/// [[3: -- If present, version MUST be v3 --
/// extensions [3] Extensions{{CertExtensions}} OPTIONAL
/// ]], ... }
/// TBSCertificate ::= SEQUENCE {
Copy link
Contributor

Choose a reason for hiding this comment

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

The brackets in comments should be escaped to make cargo doc happy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are in a text block like they should be in order to avoid this problem. No escaping is needed.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we should really add a test that cargo doc completes without warnings

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Thanks.

Copy link
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

Please add back the schema comment on TbsCertificate::version

Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
The remaining types had interrelated dependencies and needed to be
performed as a set. These types are:
  * DistributionPoint
  * DistributionPointName
  * IssuingDistributionPoint

Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
@npmccallum
Copy link
Contributor Author

@tarcieri Rather than just copy/pasting schema into the version field comment, instead I wrote a proper comment explaining the full meaning of the situation.

@tarcieri
Copy link
Member

@npmccallum fantastic!

@npmccallum
Copy link
Contributor Author

@tarcieri Ready to merge?

@tarcieri tarcieri merged commit 41643aa into RustCrypto:master Feb 11, 2022
@npmccallum npmccallum deleted the newderive branch March 10, 2022 20:46
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.

3 participants