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

[Enhancement] Add configure detection #13

Merged
merged 3 commits into from
Nov 30, 2019
Merged

Conversation

heyanlong
Copy link
Member

Add configure detection, if it does not exist, automatically skip loading.
If open ~/.skywalking.yml no such file, output warning and skip the file

@heyanlong heyanlong added the enhancement New feature or request label Nov 18, 2019
Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

I’m not sure whether we should log warning messages when the config file is absent, because it’s completely optional and is just for reusing options in command line, when the users want to just run one or two commands for a quick look with all options given in command line, the warning message breaks the output json format and forces the users to create a configuration file there, WDYT @wu-sheng

@hanahmily
Copy link
Contributor

We can send these warning logs to STDERR.If the user wants to ignore them, issue swcli ... 2>/dev/null to throw them to trash.

@wu-sheng
Copy link
Member

I think log into a file is not suitable for CLI. Recommend debug log, whicb we have an optional to open it.

@wu-sheng
Copy link
Member

This is a not issue for CLI, just helps for debuging cli itself or user env, right?

@codecov-io
Copy link

codecov-io commented Nov 20, 2019

Codecov Report

Merging #13 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #13   +/-   ##
=======================================
  Coverage   64.28%   64.28%           
=======================================
  Files           5        5           
  Lines          84       84           
=======================================
  Hits           54       54           
  Misses         24       24           
  Partials        6        6

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8091f05...2c1aeff. Read the comment docs.

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

Merging

@kezhenxu94 kezhenxu94 merged commit d5ecfb5 into apache:master Nov 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
5 participants