Added gmtoff() method in 'LogMessageTime' to get GMT offset #727

vijaysattigeri · 2021-10-20T11:30:53Z

closes #707

As per enhancement request attached above, added gmtoff() method in 'LogMessageTime' to get GMT offset.

Rearranged the code to access the variable "FLAGS_log_utc_time" in this file

coveralls · 2021-10-20T11:49:59Z

Coverage increased (+0.06%) to 76.667% when pulling 1fb5429 on vijaythreadtemp:gmt_offset_api into 8b87221 on google:master.

sergiud

Thanks for the PR. Unfortunately, there are some build errors.

src/glog/logging.h.in

src/logging_custom_prefix_gmtoffset_unittest.cc

src/logging.cc

src/logging_custom_prefix_gmtoffset_unittest.cc

vijaysattigeri · 2021-11-12T17:32:59Z

Hi @sergiud ,

I've added a unit test for my newly added code in the existing file "logging_custom_prefix_unittest.cc". But still "Coverage" check is failing.
Could you please help me here?

Thanks

sergiud · 2021-11-12T17:38:44Z

Thanks! Could you please rebase?

vijaysattigeri · 2021-11-15T06:43:51Z

Hi @sergiud ,

Rebased the branch. Please have a look.

src/logging.cc

src/logging_custom_prefix_unittest.cc

src/logging.cc

src/glog/logging.h.in

sergiud · 2021-12-06T12:00:40Z

src/glog/logging.h.in

-  const int& dayOfWeek() const { return time_struct.tm_wday; }
-  const int& dayInYear() const { return time_struct.tm_yday; }
-  const int& dst() const { return time_struct.tm_isdst; }
+  LogMessageTime() {


You can initialize all these members in constructor's initializer list.

codecov-commenter · 2021-12-07T14:48:22Z

Codecov Report

Merging #727 (298f1d5) into master (ee6faf1) will increase coverage by 0.11%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master     #727      +/-   ##
==========================================
+ Coverage   72.65%   72.76%   +0.11%     
==========================================
  Files          17       17              
  Lines        3211     3224      +13     
==========================================
+ Hits         2333     2346      +13     
  Misses        878      878

Impacted Files	Coverage Δ
src/logging.cc	`73.63% <88.09%> (-0.05%)`	⬇️
src/glog/logging.h.in	`83.13% <100.00%> (+0.85%)`	⬆️
src/windows/port.cc	`54.54% <0.00%> (+27.27%)`	⬆️

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 ee6faf1...298f1d5. Read the comment docs.

vijaysattigeri · 2021-12-08T05:21:16Z

Hi @sergiud ,

Looks like there is some issue with "VS-15-2017-x64-C++11-Release-shared-no-custom-prefix".
It says,

Runner GitHub Actions 293 did not respond to a cancelation request with 00:05:00

Is it possible to rerun this job?

Thanks,
Vijay

src/glog/logging.h.in

sergiud

This looks almost ready to me. I have only minor remarks.

You should also probably add your name to AUTHORS and CONTRIBUTORS files as per contribution guide and eventually squash and rebase onto current master (if not done yet).

src/logging.cc

src/logging_custom_prefix_unittest.cc

* Added API to get GMT offset * Made LogMessageTime as a memeber of LogMessage * Refactored LogSink::send() method

sergiud · 2021-12-09T09:26:37Z

Thanks!

vijaysattigeri · 2021-12-09T10:59:04Z

Hi @sergiud ,

This was my first open source contribution.
Thank you very much for helping me out with code reviews and suggestions.

sergiud · 2021-12-09T11:02:27Z

You're welcome. You did a great job!

google-cla bot added the cla: yes label Oct 20, 2021

sergiud suggested changes Oct 20, 2021

View reviewed changes

src/glog/logging.h.in Show resolved Hide resolved

sergiud reviewed Oct 20, 2021

View reviewed changes

src/glog/logging.h.in Outdated Show resolved Hide resolved

vijaysattigeri marked this pull request as ready for review October 20, 2021 21:30

vijaysattigeri requested a review from sergiud October 26, 2021 09:24

sergiud suggested changes Oct 28, 2021

View reviewed changes

src/logging_custom_prefix_gmtoffset_unittest.cc Outdated Show resolved Hide resolved

src/logging.cc Outdated Show resolved Hide resolved

src/logging.cc Outdated Show resolved Hide resolved

src/logging_custom_prefix_gmtoffset_unittest.cc Outdated Show resolved Hide resolved

vijaysattigeri requested a review from sergiud October 29, 2021 19:06

sergiud suggested changes Nov 15, 2021

View reviewed changes

src/logging.cc Outdated Show resolved Hide resolved

vijaysattigeri requested a review from sergiud November 18, 2021 12:52

sergiud suggested changes Nov 18, 2021

View reviewed changes

src/logging_custom_prefix_unittest.cc Outdated Show resolved Hide resolved

sergiud reviewed Nov 18, 2021

View reviewed changes

src/logging.cc Outdated Show resolved Hide resolved

vijaysattigeri commented Dec 3, 2021

View reviewed changes

src/glog/logging.h.in Show resolved Hide resolved

vijaysattigeri commented Dec 3, 2021

View reviewed changes

src/glog/logging.h.in Outdated Show resolved Hide resolved

sergiud reviewed Dec 6, 2021

View reviewed changes

vijaysattigeri requested a review from sergiud December 6, 2021 15:04

sergiud reviewed Dec 8, 2021

View reviewed changes

src/glog/logging.h.in Outdated Show resolved Hide resolved

src/glog/logging.h.in Show resolved Hide resolved

sergiud suggested changes Dec 9, 2021

View reviewed changes

src/logging.cc Outdated Show resolved Hide resolved

src/logging_custom_prefix_unittest.cc Outdated Show resolved Hide resolved

Added an API to get GMT offset from LogMessageTime

298f1d5

* Added API to get GMT offset * Made LogMessageTime as a memeber of LogMessage * Refactored LogSink::send() method

sergiud added this to the 0.6 milestone Dec 9, 2021

sergiud approved these changes Dec 9, 2021

View reviewed changes

sergiud merged commit ef36f80 into google:master Dec 9, 2021

sergiud added the enhancement label Dec 9, 2021

vijaysattigeri deleted the gmt_offset_api branch December 9, 2021 11:05

hcoona mentioned this pull request Oct 17, 2022

chore(deps): update dependency com_github_google_glog to v0.6.0 hcoona/one#65

Closed

1 task

Added gmtoff() method in 'LogMessageTime' to get GMT offset #727

Added gmtoff() method in 'LogMessageTime' to get GMT offset #727

Uh oh!

Conversation

vijaysattigeri commented Oct 20, 2021

Uh oh!

coveralls commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergiud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vijaysattigeri commented Nov 12, 2021

Uh oh!

sergiud commented Nov 12, 2021

Uh oh!

vijaysattigeri commented Nov 15, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergiud Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vijaysattigeri commented Dec 8, 2021

Uh oh!

Uh oh!

Uh oh!

sergiud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sergiud commented Dec 9, 2021

Uh oh!

vijaysattigeri commented Dec 9, 2021

Uh oh!

sergiud commented Dec 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Oct 20, 2021 •

edited

Loading

codecov-commenter commented Dec 7, 2021 •

edited

Loading