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

Gas reporter #71

Merged
merged 7 commits into from
Jan 16, 2018
Merged

Gas reporter #71

merged 7 commits into from
Jan 16, 2018

Conversation

cpurta
Copy link
Contributor

@cpurta cpurta commented Jan 12, 2018

Add eth-gas-reporter for app testing. Creating a PR in aragon-core so that the truffle-config file needed will be imported when that is merged.

Resolves #65

@cpurta
Copy link
Contributor Author

cpurta commented Jan 15, 2018

Not sure if the package-lock.json files should have been included in this PR. Seems like I should remove those from this PR.

@izqui
Copy link
Contributor

izqui commented Jan 15, 2018

Why is that?

@cpurta
Copy link
Contributor Author

cpurta commented Jan 15, 2018

Just seems like a lot of changes. Have also seen some debates in other repos to not include the lock files in commits. Guess that is just preference though.

@izqui
Copy link
Contributor

izqui commented Jan 15, 2018

Paging in @sohkai and @bpierre for a better discussion on this

Copy link
Contributor

@izqui izqui left a comment

Choose a reason for hiding this comment

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

Awesome! Please add script to package.json in every app and in the root package for running it with lerna: https://github.com/aragon/aragonOS/pull/185/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R7

@cpurta
Copy link
Contributor Author

cpurta commented Jan 16, 2018

Added the test to all apps and is enable to run by lerna. Was not able to get it to properly run due to the compilation errors that are occurring during tests.

@bpierre
Copy link
Contributor

bpierre commented Jan 16, 2018

Not sure if the package-lock.json files should have been included in this PR. Seems like I should remove those from this PR.

Hi @cpurta! Yes we are using lock files in aragon/aragon and aragon/ui, but we are prioritizing yarn.lock for now, because npm is having some issues with npm link that yarn doesn’t have.

Ideally, I think we should support both until the two projects agree on a common format, but keeping them in sync need to be done manually, which is too annoying IMO.

So, to answer the question: yes, having a package-lock.json is fine, and having a yarn.lock instead is even better if you are using yarn 😊

@cpurta
Copy link
Contributor Author

cpurta commented Jan 16, 2018

@bpierre, thanks for the clarification.

@izqui izqui merged commit e8e931e into aragon:master Jan 16, 2018
ramilexe pushed a commit to ConsiderItDone/aragon-apps that referenced this pull request Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants