-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
CORDA-1999 Changed isRelevant to relevancyStatus. #3966
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.
One minor doc change.
@@ -113,8 +113,9 @@ class NodeVaultService( | |||
// For EVERY state to be committed to the vault, this checks whether it is spendable by the recording | |||
// node. The behaviour is as follows: | |||
// | |||
// 1) All vault updates marked as RELEVANT will, of, course all have isRelevant = true. | |||
// 2) For ALL_VISIBLE updates, those which are not relevant according to the relevancy rules will have isRelevant = false. | |||
// 1) All vault updates marked as RELEVANT will, of, course all have relevancyStatus = true. |
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.
Probably want to rephrase this now that relevancyStatus
is clearly no longer a boolean (true, false).
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.
Ah.. in the DB, the relevancy flag is actually a boolean but this wording isn't very clear. I'll change it.
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.
Actually it's an INT in the db - the default type used for Enum type mappings ;-)
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.
Ah this is true :)
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.
I think the comment still needs updating (or removing).
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.
Ah whoops forgot... Just pushed up the changes now.
Over to you @josecoll ! |
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.
LGTM!
* Changed isRelevant to relevancyStatus. * Fix cash selection from breaking. * Fixed non-backwards compatible API change. * Updated schema migration changelog. * Updated comment.
Does what it says on the tin.