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

feat(storefront): strf-9921 Hide logging for successful internal API calls in Stencil CLI: #1033

Conversation

bc-max
Copy link
Contributor

@bc-max bc-max commented Nov 23, 2022

What?

At the moment we have verbose logging by default. However, Stencil-cli produces too many logs which seems to be too noisy. This PR makes non-verbose option as a default one.

!!!! This PR requires documentation update

Tickets / Documentation

Screenshots (if appropriate)

cornerstone — -zsh — 159×30 2022-11-23 16-29-06

cc @bigcommerce/storefront-team

…calls in Stencil CLI:

- verbose logging was moved to non-default option;
@bc-max bc-max force-pushed the STRF-9921-Hide-logging-for-successful-internal-API-calls-in-Stencil-CLI branch from ab411c1 to 6935b9e Compare November 23, 2022 14:21
@bc-max bc-max self-assigned this Nov 23, 2022
@bc-max bc-max requested a review from jairo-bc November 23, 2022 14:44
@bc-max bc-max marked this pull request as ready for review November 23, 2022 14:45
@@ -87,7 +87,7 @@ function checkNodeVersion() {
function applyCommonOptions(program) {
program
.option('-h, --host [hostname]', 'specify the api host')
.option('-nov, --no-verbose', 'supress verbose info logging', false)
.option('-vb, --verbose', 'enable verbose info logging', false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it considered as a breaking change and should be documented? As we are following semver, we might want to add the note to the description (or title, we need to check in semantic release docs), so the tool automatically bumps the number to 6.x.x

cc @bookernath

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think we need to update our docs. Thus I made a remark in PR description. Once the PR approved, I'll create respective ticket for documentation team.

@bc-max bc-max merged commit d8c2492 into bigcommerce:master Dec 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2022

🎉 This PR is included in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants