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

Update CI tooling and workflows (#1) #6

Closed
wants to merge 0 commits into from

Conversation

jorgecuesta
Copy link
Collaborator

Summary

  • Add Prettier
  • Add Eslint
  • Add CI GitHub workflows:
    • Add On Host CI (run things locally)
    • Add On Docker CI (run things with docker)
    • Add Cache Cleanup after PR merge
  • Add baseline for Test (but are not working)
  • Remove env variable WATCH in favor of using NODE_ENV as the driver for dependencies and execution simplifying DevEx
  • Remove old GitHub Workflows

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (CI)

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have commented my code
  • I have performed a self-review of my own code; both comments & source code
  • I create and reference any new tickets, if applicable
  • I have left TODOs throughout the codebase, if applicable

@jorgecuesta jorgecuesta added enhancement New feature or request tooling labels Jul 5, 2024
@jorgecuesta jorgecuesta self-assigned this Jul 5, 2024
Copy link
Contributor

@bryanchriswhite bryanchriswhite left a comment

Choose a reason for hiding this comment

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

Thanks for diving in @jorgecuesta - there's a lot going on here! 🦾

.env.sample Outdated
# Uncomment below env variables and comment NODE_ENV= above to run tests instead of the application
#SUB_COMMAND=test
#NODE_ENV=test

# for development purposes is better reduce the amount of worker due to the number of logs that many
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# for development purposes is better reduce the amount of worker due to the number of logs that many
# for development purposes is better reduce the amount of workers due to the number of logs that many

.env.sample Outdated
Comment on lines 19 to 21
# Uncomment below env variables and comment NODE_ENV= above to run tests instead of the application
#SUB_COMMAND=test
#NODE_ENV=test
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about moving these to a .env.test which includes ENDPOINT=http://proxy:26657? This would be tracked in git.

.env.sample Outdated
Comment on lines 15 to 17
# Drive the docker build and dependencies that will be installed on docker
# also probably is used some layer down on runtime by the code.
NODE_ENV=development
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding of conventional dotenv usage is that NODE_ENV cannot be included as it's often used to determine which .env file to load, which is a pattern I think we would benefit from here.

.env.sample Outdated
Comment on lines 32 to 34
ENDPOINT=http://proxy:26657
# Testnet
# ENDPOINT=https://testnet-validated-validator-rpc.poktroll.com
#ENDPOINT=https://testnet-validated-validator-rpc.poktroll.com
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding a .env.dev that's configured to use poktroll localnet. This can be tracked in git. The .env.sample could then set a higher worker count.

I think the .env.sample should have the Shannon testnet configured so that when a new developer follows the readme, they end with a working indexer. It is an example of realistic production environment variables.

.eslintrc.js Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
scripts/build.sh Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/mappings/utils.ts Outdated Show resolved Hide resolved
@jorgecuesta
Copy link
Collaborator Author

jorgecuesta commented Jul 17, 2024

Close in favor of creating it from another branch that is not main to rebase it with the latest commit on the upstream main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tooling] Setup Github actions CI
2 participants