-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat/rmrk2.0 #43
feat/rmrk2.0 #43
Conversation
@vikiival mind make a review for submitted code, to make sure things going on the right path and code style :) Also made very specific type for every interaction, since v2 standard has more than |
Mark as unread, will check tomorrow! |
😉 |
hey, @vikiival mind making a review? |
Yup on 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.
Nice draft!
Please move all the logic and types to folder src/rmrk/v2
@@ -9,7 +9,8 @@ export const isRemark = (text: string): boolean => { | |||
export const splitBySquare = (text: string): string[] => text.split(SQUARE) | |||
|
|||
export const isValidInteraction = (interaction: string, throwable = true): boolean => { | |||
const value = (Object.values(Interaction) as string[]).includes(interaction) | |||
const availableInteractions: string[] = [...Object.values(Interaction), ...Object.values(InteractionV2)] |
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.
Merge the object into constants
|
Signed-off-by: Junwei Chen <kngzhi@hotmail.com>
Heylo, I am integrating that into kodadot/rubick but I would be super haply if you can help us to make code review or so |
@vikiival Okay please give me some instructions on how to run this locally and I will test it |
I can publish suite to that would behave as express server. So we can test if it creates correct data.
|
this command doesnt work for me so I guess I will clone KngZhi's repo I think
Yes please publish all the ressources to test this and explain as if I was a 5yo
yes I will try to get this. Can you point me to the unit tests pls? |
rest of the work will continue in branch |
this PR is related with #42
current progress