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

Log Decorator is required by VMDB::Loggers, but not in Gemfile #16751

Merged
merged 1 commit into from
Jan 5, 2018

Conversation

juliancheal
Copy link
Member

It seems to only be required in Amazon SSA Support gem.
Meaning if that gem isn't included ManageIQ doesn't run.

@juliancheal
Copy link
Member Author

@miq-bot assign @jrafanie

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

LGTM

@juliancheal
Copy link
Member Author

Whops forgot a comma

@juliancheal juliancheal force-pushed the adding_log_decorator_gem branch 2 times, most recently from c170804 to 727a216 Compare January 5, 2018 16:07
@jrafanie
Copy link
Member

jrafanie commented Jan 5, 2018

Whops forgot a comma

Thanks @miq-bot

@juliancheal juliancheal force-pushed the adding_log_decorator_gem branch from 727a216 to 1697e54 Compare January 5, 2018 16:13
@juliancheal
Copy link
Member Author

Ah that explains it. It's the difference between Gemfiles and Gemspec files.
https://github.com/ManageIQ/amazon_ssa_support/blob/master/amazon_ssa_support.gemspec#L24

Where you can do "log_decorator", "~> 0.1" in a Gemspec but not in a Gemfile.

Gemfile Outdated
@@ -41,6 +41,7 @@ gem "highline", "~>1.6.21", :require => false
gem "inifile", "~>3.0", :require => false
gem "kubeclient", "~>2.4.0", :require => false # For scaling pods at runtime
gem "linux_admin", "~>1.2.0", :require => false
gem 'log_decorator' "~>0.1", :require => false
Copy link
Member

Choose a reason for hiding this comment

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

For consistency can you use double-quotes?

Copy link
Member

Choose a reason for hiding this comment

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

and a comma after log_decorator

Copy link
Member Author

Choose a reason for hiding this comment

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

@Fryguy ah good point. Again I just took it from the other gem. Consistency everywhere!

@Fryguy
Copy link
Member

Fryguy commented Jan 5, 2018

Also, you are still missing a comma

@juliancheal
Copy link
Member Author

juliancheal commented Jan 5, 2018

Also, you are still missing a comma

:'( oh my!

It seems to only be required in Amazon SSA Support gem.
Meaning if that gem isn't included ManageIQ doesn't run.
@juliancheal juliancheal force-pushed the adding_log_decorator_gem branch from 1697e54 to 0ac8d73 Compare January 5, 2018 16:21
@juliancheal
Copy link
Member Author

Ok final try! Sorry team!

@miq-bot
Copy link
Member

miq-bot commented Jan 5, 2018

Checked commit juliancheal@0ac8d73 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@jrafanie
Copy link
Member

jrafanie commented Jan 5, 2018

Also, you are still missing a comma

:'( oh my!

Extra u's, missing commas... what's next?

@jrafanie
Copy link
Member

jrafanie commented Jan 5, 2018

@juliancheal can you check if this needs to be backported to gaprindashvili/fine at some point?

@jrafanie jrafanie merged commit a0fb84a into ManageIQ:master Jan 5, 2018
@jrafanie jrafanie added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants