-
Notifications
You must be signed in to change notification settings - Fork 21
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/shu frontend #131
base: main
Are you sure you want to change the base?
Feat/shu frontend #131
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
package.json
Outdated
@@ -25,11 +25,12 @@ | |||
"scripts": { | |||
"start": "craco start", | |||
"build": "craco build", | |||
"build:milkomeda-testnet": "npx -y env-cmd -f ./env/milkomeda-testnet.env npm run build", | |||
"build:milkomeda-c1-testnet": "npx -y env-cmd -f ./env/milkomeda-testnet.env npm run build", |
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.
Let's get rid of the "c1" here.
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.
Why did you mark this as resolved, but the "c1" is still there?
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.
My bad, instead of changing the package.json, I changed the names of ci pipeline to fix build issues. I wasn't aware of the fleek naming convention. I will change it in the next commit.
package.json
Outdated
"build:sepolia": "npx -y env-cmd -f ./env/sepolia.env npm run build", | ||
"build:milkomeda": "npx -y env-cmd -f ./env/milkomeda.env npm run build", | ||
"build:milkomeda-c1": "npx -y env-cmd -f ./env/milkomeda.env npm run build", |
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.
... and here.
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.
Here too.
oracleContract | ||
); | ||
const coinsDetails = isShu | ||
? await getShuCoinDetails( |
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.
Djed and DjedShu are identical in relation to coin details. Do we really need this isShu ?
if statement here?
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.
This comment has not been addressed yet.
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, isShu
condition fetches coin details from getShuCoinDetails
instead of getCoinDetails
and there are considerable differences between the two
Preview ready at https://featshu-frontend.djed.codepreview.io
|
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.
@yogesh0509 , some comments from my previous review remain unaddressed. Everything else is fine.
Note that "c1" comment is important, because Fleek's configuration has been changed and is now not using the "c1" anymore. If I were to merge this PR now, our live deployments for milkomeda in Fleek might stop working.
package.json
Outdated
@@ -25,11 +25,12 @@ | |||
"scripts": { | |||
"start": "craco start", | |||
"build": "craco build", | |||
"build:milkomeda-testnet": "npx -y env-cmd -f ./env/milkomeda-testnet.env npm run build", | |||
"build:milkomeda-c1-testnet": "npx -y env-cmd -f ./env/milkomeda-testnet.env npm run build", |
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.
Why did you mark this as resolved, but the "c1" is still there?
package.json
Outdated
"build:sepolia": "npx -y env-cmd -f ./env/sepolia.env npm run build", | ||
"build:milkomeda": "npx -y env-cmd -f ./env/milkomeda.env npm run build", | ||
"build:milkomeda-c1": "npx -y env-cmd -f ./env/milkomeda.env npm run build", |
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.
Here too.
oracleContract | ||
); | ||
const coinsDetails = isShu | ||
? await getShuCoinDetails( |
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.
This comment has not been addressed yet.
Preview ready at https://featshu-frontend.djed.codepreview.io
|
No description provided.