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.

Sign up for GitHub

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

Jump to bottom

Feature/kris nova logger #326

Merged
merged 2 commits into from
Dec 17, 2018
Merged

Feature/kris nova logger #326

merged 2 commits into from
Dec 17, 2018

Conversation

christopherhein
Copy link
Contributor

@christopherhein christopherhein commented Nov 28, 2018 •
edited
Loading

Description

Changing logger to kris-nova/logger and using the new Timestamps option to turn off Timestamps by default.

Also easter egg… adds --fabulous to use the Fabulous mode ;) @kris-nova

Checklist

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All tests passing (i.e. make test)
  • Added/modified documentation as required (such as the README)
  • Added yourself to the humans.txt file

screen shot 2018-11-27 at 7 05 09 pm

I'm not a huge fan of having to set -C false to make --fabulous mode work, but I think for now it works.

Closes #325

@christopherhein christopherhein added kind/feature New feature or request and removed kind/feature New feature or request labels Nov 28, 2018
richardcase
richardcase previously approved these changes Dec 3, 2018
View reviewed changes
Copy link
Contributor

@richardcase richardcase left a comment

Choose a reason for hiding this comment

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

LGTM.

I like defaulting to not having timestamps show with the log entries.

@christopherhein
Copy link
Contributor Author

christopherhein commented Dec 3, 2018 •
edited
Loading

@errordeveloper hope you are enjoying the vacay, when you get back would love your input :)

errordeveloper
errordeveloper previously requested changes Dec 5, 2018
View reviewed changes
cmd/eksctl/main.go Outdated Show resolved Hide resolved
cmd/eksctl/main.go Outdated Show resolved Hide resolved
pkg/cfn/manager/nodegroup.go Show resolved Hide resolved
@christopherhein christopherhein force-pushed the feature/kris-nova-logger branch 2 times, most recently from 5aec3f4 to 6ee447f Compare December 11, 2018 06:38
@christopherhein
Adding kris-nova/logger instead of kubicorn/logger
a871568 
Signed-off-by: Christopher Hein <me@christopherhein.com>
@christopherhein
Adding vendored changes
0b32f51 
Signed-off-by: Christopher Hein <me@christopherhein.com>
@christopherhein christopherhein force-pushed the feature/kris-nova-logger branch from 6ee447f to 0b32f51 Compare December 11, 2018 06:41
@christopherhein
Copy link
Contributor Author

Cool tests pass, let me know if you want anything done differently. @errordeveloper

@errordeveloper errordeveloper merged commit d5fd907 into eksctl-io:master Dec 17, 2018
@christopherhein christopherhein deleted the feature/kris-nova-logger branch December 17, 2018 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@errordeveloper errordeveloper errordeveloper approved these changes

@richardcase richardcase richardcase left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christopherhein @richardcase @errordeveloper