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

[partial][common] log #60

Merged
merged 12 commits into from Dec 18, 2016

Conversation

Projects
None yet
3 participants
@at15
Copy link
Member

at15 commented Dec 16, 2016

Related #59

It is basic a clone of logrus with the following modification

  • fatal level has lower number than painc level (reversed in logrus)
  • remove all the Debug, Warn in Logger, only keep those methods on Entry
  • field value can only be string, logrus use interface{}
  • didn't use lock (though there is a mutex in Logger)
  • does not support hook, only have filter
  • does not trim \n for *f functions
  • add syntax sugar to create entry with pkg i.e. logger.NewEntryWithPkg("x.tsdb.kairosdb")

the more detail description can be found in README, all the logrus usage are replaced with common/log, and since WithField is not supported, it is replaced by *f....

@arrowrowe

This comment has been minimized.

Copy link
Member

arrowrowe commented Dec 16, 2016

Hey maybe you would like this: https://about.pullapprove.com/

always_pending:
  title_regex: 'WIP'
  explanation: 'Work in progress...'
@at15

This comment has been minimized.

Copy link
Member Author

at15 commented Dec 16, 2016

@arrowrowe hey maybe I should put mie on fire and brush some oil @mingo-x

@arrowrowe

This comment has been minimized.

Copy link
Member

arrowrowe commented Dec 17, 2016

@at15 you didn't set the wip rule...

at15 added a commit that referenced this pull request Dec 17, 2016

Add .pullapprove.yml
as mentioned by @arrowrowe in #60

@at15 at15 referenced this pull request Dec 17, 2016

Merged

Add .pullapprove.yml #61

at15 added some commits Dec 16, 2016

[common][log] Init filter
- Based on Logrus's code
- Add Logger
- Add Entry
- Add Filter
- Add PkgFilter
[common][log] Add log for Entry
- Add Formatter interface and TextFormatter
- Use StdOut and TextFormatter as default
- Apply filter when log

@at15 at15 force-pushed the common-util/log branch from a40f76a to 1a07fe9 Dec 17, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 17, 2016

Coverage Status

Coverage increased (+1.7%) to 59.208% when pulling 1a07fe9 on common-util/log into 0674584 on master.

at15 added some commits Dec 17, 2016

[common][log] Add logging method to Entry
- Add `Trace(f), Debug(f) ... Fatal(f)` to `Entry`
- Fix map not initialized problem for `Entry`
- Add lost `WarnLevel` for `Level.String()`
[common][log] Move Fatal before Panic
- Fatal should be the most severe log level since it simple stop the
  process, while Panic can be recovered
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 17, 2016

Coverage Status

Coverage decreased (-2.2%) to 55.316% when pulling b2bfbb3 on common-util/log into 0674584 on master.

[common][log] Support timestamp and color
- Found a bug in logrus's `minTS` when copy and paste, should not use
  current time to substract base time, should use `entry.Time`
- Support color, though, logrus's color is not very ... pretty
- Support short and full/custom timestamp
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 18, 2016

Coverage Status

Coverage increased (+1.0%) to 58.488% when pulling 35de1ad on common-util/log into 0674584 on master.

[common][log] Fix typo Painc -> Panic
Found it when writing xephon-b
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 18, 2016

Coverage Status

Coverage increased (+1.0%) to 58.488% when pulling 0ef0065 on common-util/log into 0674584 on master.

at15 added some commits Dec 18, 2016

[common][log] Add NewEntryWithPkg
- Rename test to IDEA style `TestStruct_Method`
- Add comment to some expored methods and structs
[common][log] Replace logrus with common/log
- TODO: WithField is useful when u want to add extra fields
- TODO: Tags are shown on the next line when run `Ayi test`, it has no
  problem when running unit test in `TestLogger_NewEntryWithPkg`
[util][runner] Remove trailing \n for log
Fix the problem mentioned by previous commit, I guess logrus would
trim the trailing `\n`
[common][log] Add doc in README
- TODO: fill the doc and example for doc.go
- Compare with logrus
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 18, 2016

Coverage Status

Coverage increased (+1.5%) to 58.989% when pulling c611a4c on common-util/log into 0674584 on master.

[lib] Move testify to testimport
- bump testify to latest commit
- remove commented packages

@at15 at15 added the type/backlog label Dec 18, 2016

@at15 at15 changed the title [WIP][common] log [partial][common] log Dec 18, 2016

@at15

This comment has been minimized.

Copy link
Member Author

at15 commented Dec 18, 2016

LGTM

@at15

This comment has been minimized.

Copy link
Member Author

at15 commented Dec 18, 2016

LGTM

Approved with PullApprove

@at15 at15 merged commit b357803 into master Dec 18, 2016

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
code-review/pullapprove Approved by at15
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 58.989%
Details
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 18, 2016

Coverage Status

Coverage increased (+1.5%) to 58.989% when pulling ddab70d on common-util/log into 0674584 on master.

@at15 at15 deleted the common-util/log branch Apr 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment