Skip to content

Make use of server timezone API when possible#26

Merged
wu-sheng merged 3 commits intomasterfrom
feature/timezone
Dec 31, 2019
Merged

Make use of server timezone API when possible#26
wu-sheng merged 3 commits intomasterfrom
feature/timezone

Conversation

@kezhenxu94
Copy link
Copy Markdown
Member

@kezhenxu94 kezhenxu94 commented Dec 28, 2019

@kezhenxu94 kezhenxu94 added the enhancement New feature or request label Dec 28, 2019
@kezhenxu94 kezhenxu94 added this to the 0.2.0 milestone Dec 28, 2019
@kezhenxu94 kezhenxu94 requested a review from wu-sheng December 28, 2019 09:30
@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 28, 2019 •
edited
Loading

Codecov Report

Merging #26 into master will decrease coverage by 10.25%.
The diff coverage is 10.52%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #26       +/-   ##
===========================================
- Coverage    64.7%   54.45%   -10.26%     
===========================================
  Files           5        6        +1     
  Lines          85      101       +16     
===========================================
  Hits           55       55               
- Misses         24       39       +15     
- Partials        6        7        +1
Impacted Files Coverage Δ
commands/interceptor/timezone.go 0% <0%> (ø)
commands/interceptor/duration.go 50.98% <25%> (-5.55%) ⬇️

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 5d2b49d...a11773a. Read the comment docs.

wu-sheng
wu-sheng reviewed Dec 28, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

One suggestion, and please update the documents. Codes look good to me.

wu-sheng
wu-sheng reviewed Dec 28, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Parameter timezone should be added in the readme, right? And please add a Use Case about the TZ.

@kezhenxu94 kezhenxu94 requested a review from wu-sheng December 31, 2019 13:00
wu-sheng
wu-sheng approved these changes Dec 31, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

LGTM. We could consider adding e2e for CLI by using the latest release of SkyWalking docker image or binary tar.

@wu-sheng wu-sheng merged commit 9e3365a into master Dec 31, 2019
@wu-sheng wu-sheng deleted the feature/timezone branch December 31, 2019 13:05
@kezhenxu94
Copy link
Copy Markdown
Member Author

kezhenxu94 commented Dec 31, 2019

LGTM. We could consider adding e2e for CLI by using the latest release of SkyWalking docker image or binary tar.

Sounds good to me

@kezhenxu94 kezhenxu94 mentioned this pull request Jan 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wu-sheng wu-sheng wu-sheng approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

0.2.0

Development

Successfully merging this pull request may close these issues.

[CLI] Make use of TimeZone API when possible

3 participants

@kezhenxu94 @codecov-io @wu-sheng