Skip to content
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.

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

Add test to ensure issuanceDate is added to VC 1.1 VCs. #166

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

aljones15
Copy link
Contributor

No description provided.

@aljones15 aljones15 self-assigned this Dec 6, 2023
Copy link
Contributor

@JSAssassin JSAssassin left a comment

Choose a reason for hiding this comment

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

Just some minor nits.

test/10-verify.spec.js Outdated Show resolved Hide resolved
Comment on lines 197 to 198
const jsonDate = now.toJSON();
const expectedIssuanceDate = `${jsonDate.slice(0, jsonDate.length - 5)}Z`;
Copy link
Contributor

@JSAssassin JSAssassin Dec 6, 2023

Choose a reason for hiding this comment

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

Just another way of doing it, but one less line.

Suggested change
const jsonDate = now.toJSON();
const expectedIssuanceDate = `${jsonDate.slice(0, jsonDate.length - 5)}Z`;
const expectedIssuanceDate = now.toISOString().slice(0, -5) + 'Z';

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the now.toISOString() approach better, but we should keep the string interpolation in place.

Copy link
Contributor Author

@aljones15 aljones15 Dec 6, 2023

Choose a reason for hiding this comment

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

@JSAssassin @BigBlueHat I like the toISOString method more too, I was copying the way the issuanceDate is set inside of index.js should I update that to? test updated here: b34181d

p.s. I could update the one in index.js to use toISOString.slice(0, -5) in the validUntil PR as that already contains breaking changes to the DateRegexp.

Copy link
Contributor

@BigBlueHat BigBlueHat left a comment

Choose a reason for hiding this comment

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

Agree with @JSAssassin. Those small tweaks would be nice.

@aljones15 aljones15 merged commit 0fc159b into main Dec 6, 2023
5 checks passed
@aljones15 aljones15 deleted the add-issuanceDate-added-test branch December 6, 2023 21:40
@aljones15 aljones15 mentioned this pull request Dec 7, 2023
12 tasks
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