Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Core: add managed ganache version info to the output of truffle version #4686

Merged
merged 2 commits into from
Feb 4, 2022

Conversation

lsqproduction
Copy link
Contributor

Resolved issue #4678 by reading from Truffle's package.json for Ganache's version number.
Once Ganache adds .version, we can make a change to use that instead.

Also, BUNDLE_VERSION is causing an eslint no-undef rule complaint, this variable is now on the package level eslintrc.json, should this be moved to global eslintrc.json?

@haltman-at
Copy link
Contributor

Also, BUNDLE_VERSION is causing an eslint no-undef rule complaint, this variable is now on the package level eslintrc.json, should this be moved to global eslintrc.json?

I think either way makes sense, but let @cds-amal know if you're doing the latter, because he's also dealing with this on #4671.

Copy link
Contributor

@haltman-at haltman-at left a comment

Choose a reason for hiding this comment

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

Looks good to me! Might want to check about what to do about BUNDLE_VERSION before merging though.

@cds-amal
Copy link
Member

cds-amal commented Feb 3, 2022

Looks good to me! Might want to check about what to do about BUNDLE_VERSION before merging though.

#4671 uses BUNDLE_CHAIN_FILENAME but BUNDLE_VERSION is used in three places, resolver, core, and truffle (for webpack). I don't think this warrants moving up to global, but also don't feel strongly about that position.

Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @lsqproduction !

@haltman-at
Copy link
Contributor

Oh, I missed the distinction there, thanks! In that case my opinion is that it should probably be moved up to global. I don't know that it makes a big difference though.

"extends": ["../../.eslintrc.package.json"]
"extends": ["../../.eslintrc.package.json"],
"globals": {
"BUNDLE_VERSION":"readonly"
Copy link
Contributor

Choose a reason for hiding this comment

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

just a nit, can you add a space after the :?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okie thank you for catching it.

@@ -60,9 +60,15 @@ const logWeb3 = (logger = console) => {
logger.log(`Web3.js v${web3Version}`);
};

const logGanache = (logger = console) => {
const ganacheVersion = pkg.dependencies.ganache;
logger.log(`Ganache V${ganacheVersion}`);
Copy link
Contributor

@eggplantzzz eggplantzzz Feb 3, 2022

Choose a reason for hiding this comment

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

one more nit, how would you feel about making the V lowercase so it matches the format that we print web3's version in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okie I will make the change.

@lsqproduction
Copy link
Contributor Author

lsqproduction commented Feb 4, 2022

Looks good to me! Might want to check about what to do about BUNDLE_VERSION before merging though.

#4671 uses BUNDLE_CHAIN_FILENAME but BUNDLE_VERSION is used in three places, resolver, core, and truffle (for webpack). I don't think this warrants moving up to global, but also don't feel strongly about that position.

I will keep it at the package level then.

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

Successfully merging this pull request may close these issues.

4 participants