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

add logrus template. #4

Closed
wants to merge 1 commit into from

Conversation

trusch
Copy link
Contributor

@trusch trusch commented Nov 1, 2018

This adds a template similar to the log template but uses logrus with field annotations for

  • component (name of the interface)
  • method
  • parameters
  • results

This makes it easier to retrieve these values out of the structured output of logrus' formats.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.365%

Totals Coverage Status
Change from base Build 12: 0.0%
Covered Lines: 197
Relevant Lines: 211

💛 - Coveralls

@hexdigest
Copy link
Owner

hexdigest commented Nov 1, 2018

HI, @trusch

First of all thank you for your PR!

Here are my thoughs:

  1. I think in case of logrus it makes sence to pass logrus.Entry as a parameter to the constructor. This will allow users to configure whatever way they want.
  2. I think that it makes sence to use logrus.Error/logrus.Errorf methods in case of errors, please check {{$method.ReturnsError}} section in the original "log" template
  3. Please generate decorator with your new template for the TestInterface (templates_tests/interface.go). So that we can be sure that generated code is valid.
  4. Also I notices incorrect comment for the constructor (which is my bad and I'll fix it in the "log" template). It should be something like:
  // New{{$decorator}} instruments an implementation of the {{.Interface.Type}} with logrus logger

Do you have time to fix above issues?

@hexdigest
Copy link
Owner

@trusch Can you please check the logrus template I just added? What would you add/change?

@trusch
Copy link
Contributor Author

trusch commented Nov 5, 2018

@hexdigest looks good to me! Thank you very much for this template and the project in general!

@trusch trusch closed this Nov 5, 2018
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.

None yet

3 participants