Conversation
|
And there is a bug Fixed |
|
There is a new bug:
It's there: It's also the very first tried Fixed! |
mirror/mirror.go
Outdated
| return fmt.Errorf("building tree: %w", err) | ||
| } | ||
|
|
||
| // TODO Would the spec allow a small NotAfter? |
There was a problem hiding this comment.
I don't see any reason for the spec to disallow small NotAfter values, although it should be the CA's right to require some minimum validity for entries it issues.
There was a problem hiding this comment.
It was a typo. Corrected.
There was a problem hiding this comment.
I don't see the corresponding change--did you push?
There was a problem hiding this comment.
Would the spec allow a NotAfter before batchStart?
| } | ||
|
|
||
| if h.b.Params.EvidencePolicy != mtc.UmbilicalEvidencePolicy { | ||
| continue |
There was a problem hiding this comment.
Maybe move everything else in this loop below to a separate ValidateUmbilicalEvidence function that gets called here?
There was a problem hiding this comment.
It's a bit of a long function. Because of the shared state, it's not that easy to pull into a function: we'd need an object.
lukevalenta
left a comment
There was a problem hiding this comment.
Only minor nits left. Feel free to merge when ready!
All functionality is there—just need to test it.
Note that this includes a backwards incompatible change to EvidencePolicyType (adding a zero value.)