-
Notifications
You must be signed in to change notification settings - Fork 63
LW-9675 conway era fixes #1480
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
LW-9675 conway era fixes #1480
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, as usual 🧙♂️
@@ -1,4 +1,4 @@ | |||
// cSpell:ignore descr | |||
// cSpell:ignore descr timelock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove all cSpell comments or set it up as a check in the repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cSpell is a VSCode extension. What is the problem?
Those who have it installed, can get benefit from it; those who don't, are not impacted by the comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem is that it's specific to your dev environment. These comments don't make sense for the repo in general. My suggestion here is to either:
- add a "cspell" npm script and run it in CI
- or don't commit those comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I understand it's relative to my dev environment, but I can't see which problem those comments are originating.
Yes, it could be a good idea to add a repository level additional check, but it is OT from this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok let's make a deal: keep it but follow-up with a new PR to add this check :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and that is what I was doing, but that means every developer need to apply the same ignore by themselves... multiple times, on each distinct development environment they use.
export interface RedeemerModel { | ||
index: number; | ||
purpose: 'cert' | 'mint' | 'spend' | 'reward' | 'voting' | 'proposing'; | ||
purpose: 'cert' | 'mint' | 'spend' | 'reward' | 'vote' | 'propose'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IDK, I didn't checked...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for 71f6de7 commit then? Which db-sync version it is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
db-sync version is 13.5.0.1
I taken the values directly from the DB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
13.5.0.1 tagged commit also has voting
and proposing
in docs. Let's report to db-sync devs
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/util.ts
Outdated
Show resolved
Hide resolved
4dafe83
to
e99b89e
Compare
e99b89e
to
8f1ca36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🚀
Context
Data present in sanchonet let us to discover some minor issues on something we did early about Conway era.
Proposed Solution
Details in the commits.