Merged
Conversation
added 7 commits
June 25, 2020 17:12
… nicaea-types-2nd-update
mnaamani
approved these changes
Jun 29, 2020
Member
mnaamani
left a comment
mnaamani
left a comment
There was a problem hiding this comment.
Left some comments but those can be tackled in separate PR. Not critical.
|
|
||
| if (!optLead.isSome) { | ||
| return null; | ||
| if (!optLeadId.isSome) { |
Member
There was a problem hiding this comment.
I've done similar checks, but I realize now its more straightforward to test optLeadId.isNone
|
|
||
| if (!profile) { | ||
| throw new Error(`Group lead profile not found! (member id: ${lead.member_id.toNumber()})`); | ||
| if (!leadWorker.is_active) { |
Member
There was a problem hiding this comment.
We should really output some error message here, it would indicate invalid state in the runtime to have a set lead and the worker not existing.
| ); | ||
| const leadWorker = leadWorkerLink.value; | ||
|
|
||
| if (!leadWorker.is_active) { |
Member
There was a problem hiding this comment.
again if this check fails, there is bad state in the chain, and we should log it at least.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR (currently a draft) intoduces changes to
@joystream/types, Pioneer and CLI that should make those compatible with the runtime build after #728.Related issue: #816
Current commit I was basing the changes on: 960abce
Updates to #707 and #742 will also be required (already did them locally to properly test if everything works)
Besides that I'm not introducing any UI adjustments related to the fact that lead is hireable yet, so:
There is a separate issue related to that (#726) which I'm going to work on later.