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

fix: replace Duration with u64 in schema #4841

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Jul 12, 2024

Description

  • replace Duration with u64 in schema.json
  • enable registering time trigger in genesis

Linked issue

Closes #{issue_number}

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Jul 12, 2024
@mversic mversic force-pushed the time_trigger_fix branch from e8260e3 to cade53a Compare July 12, 2024 11:20
@Erigara
Copy link
Contributor

Erigara commented Jul 12, 2024

What is your motivation for removing Duration from schema?

@mversic
Copy link
Contributor Author

mversic commented Jul 12, 2024

What is your motivation for removing Duration from schema?

because it was a pain to use and incorrect. In schema we had an entry:

"Duration": {                                                          
  "Tuple": [                                                           
    "u64",                                                             
    "u32"                                                              
  ]                                                                    
}

but in genesis.json it was:

"filter": {
  "Time": {
    "Schedule": {
      "start": {
        "secs": 1790774026,
        "nanos": 0
      },
      "period": {
        "secs": 1,
        "nanos": 0
      }
    }
  }
}

we can consider restoring it but it has to be done properly

Erigara
Erigara previously approved these changes Jul 15, 2024
nxsaken
nxsaken previously approved these changes Jul 15, 2024
@nxsaken nxsaken force-pushed the time_trigger_fix branch from cade53a to 4c14031 Compare July 15, 2024 11:18
@mversic mversic enabled auto-merge (squash) July 15, 2024 11:19
dima74
dima74 previously approved these changes Jul 15, 2024
core/src/smartcontracts/isi/triggers/mod.rs Outdated Show resolved Hide resolved
@mversic mversic dismissed stale reviews from dima74, Erigara, and nxsaken via dca010e July 15, 2024 14:49
@mversic mversic force-pushed the time_trigger_fix branch from 4c14031 to dca010e Compare July 15, 2024 14:49
@mversic mversic merged commit 059c0c6 into hyperledger-iroha:main Jul 15, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants