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

feature: friendly logging for devOps #995

Closed
wants to merge 5 commits into from
Closed

feature: friendly logging for devOps #995

wants to merge 5 commits into from

Conversation

nabi-chan
Copy link

@nabi-chan nabi-chan commented Jul 16, 2021

What has been implemented?

The Pull Request adds features related to overall support for CI/CD and devOps.

Closes #834

Steps to verify

  • disable persistent status bar in the CLI
  • Add the ability to automatically detect environment variables and apply CI mode.

- [ ] Add output for serverless deploy and serverless info commands

Todos:

  • Write / update documentation
  • Run Prettier

This mode disables the persistent status bar.
@nabi-chan
Copy link
Author

@ole3021 @medikoo Hello! May I ask you to review this PR?

@ole3021
Copy link
Contributor

ole3021 commented Jul 19, 2021

@eahefnawy @medikoo could you please check this PR and follow up. this will add a new arg --ci to keep the log in ci tidy. i dont think --ci is a good name here, but the idea behind is great, which is to make the terminal logs as simple as possible (with just deploy results)

@nabi-chan
Copy link
Author

nabi-chan commented Jul 19, 2021

Thank you for your feedback. @ole3021
I'm thinking of these names right now, so could you choose between them or put them on the thread if you have any good ideas?

  • --tidy-logs
  • --clean
  • --reduce
  • --refine
  • --light
  • --no-verbose
  • --silent
  • --quiet

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

I think best solution would be to rely on feature detection, and do not introduce any option at all.

Animated progress definitely should not be shown in non TTY terminals (and by nature CI runs happen in those).

Therefore I would simply not show progress if !process.stdin.isTTY. https://nodejs.org/api/tty.html#tty_tty

@nabi-chan nabi-chan requested a review from medikoo July 19, 2021 12:03
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@KimPinot thanks for update.

Still I think more natural would be to have _isTTY and not _isNotTTY (?)

@nabi-chan nabi-chan requested a review from medikoo July 20, 2021 02:39
@nabi-chan
Copy link
Author

I thought it was mostly used when it wasn't isTTY, so I wrote it in negative form, but I guess it wasn't. 😅

@nabi-chan nabi-chan changed the title [WIP] feature: Add CI mode [WIP] feature: friendly logging for devOps Jul 20, 2021
medikoo
medikoo previously approved these changes Jul 20, 2021
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @KimPinot! Looks good to me 👍

@ole3021
Copy link
Contributor

ole3021 commented Aug 6, 2021

@medikoo are we good to process this PR?

@nabi-chan
Copy link
Author

@ole3021 Yes. The features scheduled for #945 would like to work with a different PR.

@nabi-chan
Copy link
Author

@ole3021 Can you run Lint & Formatting workflows? I forgot run prettier at CLI.js :<

@nabi-chan nabi-chan changed the title [WIP] feature: friendly logging for devOps feature: friendly logging for devOps Aug 31, 2021
@herrlegno
Copy link

Will this be merged at some point? so this workaround won't be necessary?

Thank you.

@nabi-chan
Copy link
Author

Will this be merged at some point? so this workaround won't be necessary?

Thank you.

Perhaps the form of PR you mentioned will be more helpful in addressing this goal!
I think we need to close the issue. Thank you for pointing it out. ❤️

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.

Logging suitable for CI mode must be supported.
4 participants