fix(exp): document expirationTimestamp

energywebfoundation / iam-client-lib

TypeScript library to be used within decentralised applications for authentication and authorisation using DIDs (Decentralised Identifiers) and VCs (Verifiable Credentials)

GNU General Public License v3.0

19 stars 9 forks source link

fix(exp): document expirationTimestamp #631

Closed whitneypurdum closed 2 years ago

whitneypurdum commented 2 years ago

Description

Updates documentation for expirationTimestamp to be explicit that the timestamp is in milliseconds

Contributor checklist

[x] Breaking changes - check for any existing interfaces changes that are not backward compatible, removed method etc.
[x] Documentation - document your code, add comments for method, remember to check if auto generated docs were updated.
[x] Tests - add new or updated existed test for changes you made.
[x] Migration guide - add migration guide for every breaking change.
[x] Configuration correctness - check that any configuration changes are correct ex. default URLs, chain ids, smart contract verification on Volta explorer or EWC explorer.

jrhender commented 2 years ago

Actually, I see that a Unix timestamp is in seconds not miliseconds, so we can't say that is directly a Unix timestamp. Perhaps we should say that it is a "Unix timestamp expressed in milliseconds"?

jrhender commented 2 years ago

@whitneypurdum In this PR, @JGiter is using seconds as the unit for RegisterOnchainOptions which I think might be correct https://github.com/energywebfoundation/iam-client-lib/pull/636/files What are your thoughts?

whitneypurdum commented 2 years ago

@jrhender so just in registerOnChainClaim Options is it seconds?

whitneypurdum commented 2 years ago

@whitneypurdum In this PR, @JGiter is using seconds as the unit for RegisterOnchainOptions which I think might be correct https://github.com/energywebfoundation/iam-client-lib/pull/636/files What are your thoughts?

updated so RegisterOnChainOptions is in seconds.

jrhender commented 2 years ago

@whitneypurdum I'm sorry, I'm looking at it again in and I think maybe it should be milliseconds in RegisterOnchainOptions and it is wrong in https://github.com/energywebfoundation/iam-client-lib/pull/636/files Maybe we merge with milliseconds in this PR and then we can debate seconds vs milliseconds in the other PR 😄

whitneypurdum commented 2 years ago

@whitneypurdum I'm sorry, I'm looking at it again in and I think maybe it should be milliseconds in RegisterOnchainOptions and it is wrong in https://github.com/energywebfoundation/iam-client-lib/pull/636/files Maybe we merge with milliseconds in this PR and then we can debate seconds vs milliseconds in the other PR 😄

OKay @jrhender , I changed it back to milliseconds. We can't merge anyways till we figure out why the tests are failing. This PR doesn't have the updated packages and it is now failing. Maybe something wrong with cache server?

whitneypurdum commented 2 years ago

@jrhender Please can you review this one more time?

ewf-devops commented 2 years ago

:tada: This PR is included in version 6.0.1-alpha.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

ewf-devops commented 2 years ago

:tada: This PR is included in version 6.0.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: