Skip to content
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

[EPIC] E2E DevOps #911

Open
wants to merge 13 commits into
base: staging
Choose a base branch
from
Open

[EPIC] E2E DevOps #911

wants to merge 13 commits into from

Conversation

0xlinus
Copy link
Collaborator

@0xlinus 0xlinus commented Jan 19, 2023

No description provided.

@vercel
Copy link

vercel bot commented Jan 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
aelin-ui-v2 ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 2, 2023 at 5:15PM (UTC)

@0xlinus 0xlinus self-assigned this Jan 19, 2023
@0xlinus 0xlinus added the Epic label Jan 19, 2023
package.json Outdated Show resolved Hide resolved
@@ -1,31 +1,83 @@
# Aelin Frontend v2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create a TESTING.md file to add this? I don't think it's necessary to run the entire protocol locally for development reasons. I don't think it's worth adding so much complexity to just run the app.

Copy link
Collaborator Author

@0xlinus 0xlinus Mar 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. But if there's gonna be a pool/deal creation or interaction, then we should use this. Otherwise will be continue to pollute Goerli. Also we tend to always use small token values bc of balance limitations which ended up creating bugs in the past. So:

  • If it's just to display data => no need to use the whole app
  • Need to create a pool/deal, or invest, vest etc => I'd definitely push to use this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It adds too much complexity to run the dapp IMO. I would move this to TESTING.md, which is what it was created for. I don't want new contributors to struggle w/this when the steps are now pretty simple.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still think that we should go this way:

  • If it's just to display data => no need to use the whole app
  • Need to create a pool/deal, or invest, vest etc => I'd definitely push to use this

We can discuss this later in standies

PS: running 2 commands is adding too much complexity?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it not only adds two commands, but requires installing and managing docker and docker compose. Having been developed with the docker stack, it has disadvantages compared to the previous approach. For instance: slower nextjs fast refresh, need to prune your images/container because it uses huge disk space, slower startup time...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants