-
Notifications
You must be signed in to change notification settings - Fork 795
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
feat(cli): [STENCIL-33] Writing and Reading the Ionic config file #2963
feat(cli): [STENCIL-33] Writing and Reading the Ionic config file #2963
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One overarching question - can we get these helper functions covered by (unit) tests please? LMK if you have any questions on that, although I hardly consider myself an expert 😉
@rwaskiewicz Added 6 tests for these new methods. |
Co-authored-by: Ryan Waskiewicz <[email protected]>
Co-authored-by: Ryan Waskiewicz <[email protected]>
Co-authored-by: Ryan Waskiewicz <[email protected]>
Co-authored-by: Lars Mikkelsen <[email protected]>
…gleton to help facilitate accessing features during the CLI's lifecycle.
@rwaskiewicz and @ltm if you could take a peek at the new changes in 2dd693f regarding using the abstracted system calls that help us work in Deno or Node I'd really appreciate it! I'm going to bring these over to the STENCIL 12 PR in the meantime. |
Co-authored-by: Ryan Waskiewicz <[email protected]>
2888409
to
8a40ef6
Compare
src/cli/telemetry/helpers.ts
Outdated
tty: process.stdout.isTTY ? true : false, | ||
ci: | ||
['CI', 'BUILD_ID', 'BUILD_NUMBER', 'BITBUCKET_COMMIT', 'CODEBUILD_BUILD_ARN'].filter(v => !!process.env[v]) | ||
.length > 0 || process.argv.includes('--ci'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
process
is a Node built-in, so you'd have to access these through the compiler system. You should be able to replace process.env[v]
with CompilerSystem.getEnvironmentVar(v)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, hmm... I'm going to need to do a couple changes here in that case.
Adds ability to allow writing and reading to the ~/.ionic/config.json file. When reading, if the file doesn't exist, readConfig will automatically create the file.
Satisfies ADR-2