search

cgewecke / eth-gas-reporter

Gas usage per unit test. Average gas usage per method. A mocha reporter.
MIT License
603 stars 94 forks source link

@codechecks/client should be dev dependency #229

Closed cruzdanilo closed 7 months ago

cruzdanilo commented 4 years ago

currently, @codechecks/client is a peer dependency, which causes warnings on npm/yarn install when not being used.

cgewecke commented 4 years ago

@cruzdanilo It is a dev dependency here...

https://github.com/cgewecke/eth-gas-reporter/blob/13c3e19487eb38cc0c92ab99aafc48f5626a9623/package.json#L53-L54

Do you think it should be a "real" dependency?

The engineer who writes codechecks suggested the current format while reviewing the PR which added this feature. This makes sense from the codechecks standpoint since a project might have many checks that do different things.

Things are slightly weird here because this module isn't a dedicated "check"... e.g codechecks is an option.

cgewecke commented 4 years ago

Completely decoupling the check was another suggested option...

https://github.com/cgewecke/eth-gas-reporter/pull/131#issuecomment-502873774

cruzdanilo commented 4 years ago

decoupling would be the best. if not decoupled, i think it would be better to list it in optionalDependencies. i personally don't like optionalDependencies as they are not easy to opt-out, but it makes more sense semantically and doesn't cause warnings on npm/yarn install.

cgewecke commented 4 years ago

decoupling would be the best

Tbh I didn't do it this way because it's more convenient to maintain/develop it in the same repo. The user would also have to install two additional things to run the feature...(which is fine I guess)

Optional dependencies sounds good for this case.

i personally don't like optionalDependencies as they are not easy to opt-out,

Could you explain this a bit further? What problems do you run into?

cruzdanilo commented 4 years ago

i personally don't like optionalDependencies as they are not easy to opt-out,

Could you explain this a bit further? What problems do you run into?

there is no way to select which optional dependencies you want to skip, or even from which package. there is only the --no-optional option for the whole install process.

cgewecke commented 4 years ago

Ah I see! Ok that makes sense.

cgewecke commented 7 months ago

Hi! eth-gas-reporter is being deprecated in favor of hardhat-gas-reporter.

In the latest version at Hardhat, Codechecks support has been removed because they haven't been accepting new users for a while.

https://github.com/cgewecke/hardhat-gas-reporter/releases/tag/v2.0.0