-
Notifications
You must be signed in to change notification settings - Fork 521
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
External evaluation of Dotflow Milestone 1 #888
Conversation
@dastansam Thanks a lot for your evaluation! I have added instructions regarding contract deployment and frontend environment variable. I have also resolved all the clippy errors, thanks for pointing that out. Regarding The reason why we use |
hey @Szegoo, thanks for the swift feedback.
yeah, I went through it again and you are right, // instead of this
ensure!(self.number_to_identity.get(receiver).is_some(), Error::IdentityDoesntExist);
let receiver_identity = self.number_to_identity.get(receiver).unwrap();
// do this: it's one line and one storage read less :)
let receiver_identity = self.number_to_identity.get().map_or(Error::IdentityDoesntExist, |v| Ok(v))?;
I think in general, it's not that critical to use But overall, again, I don't see any critical issues that needs to be addressed. UPDATE: updated the code to use |
@dastansam Yes, you are right. It is a good idea to refactor the code the way you described. Thanks for your suggestion. TheDotflow/dotflow-ink#38 |
It's good to go for me, let's wait for @keeganquigley's evaluation |
Thanks for the evaluation @dastansam excellent work! @Szegoo I agree, everything looks great and I am able to reproduce the results. A couple of comments:
Thanks! |
@keeganquigley changed the status of the evaluation to |
Thanks a bunch @dastansam I also made a minor title fix to adhere to our naming convention. I will forward your KSM address to our operations team to process your payment. |
hi @dastansam we just transferred the KSM |
External evaluation for the Dotflow Milestone 1