Fix SumologicLogging #180
Fix SumologicLogging #180
Conversation
The class for SumologicLogging was declared, but it was never included in fastly.rb to autogenerate the methods to call anything. I added a test, but didn't manually update the comment documentation in fastly.rb, as that looks hand generated and out of date for arguments, so I figured the test was more useful.
There was a problem hiding this comment.
Thanks @mmrobins for opening this PR, it looks OK to me (I only had one minor comment). Although I've approved, I have also requested @conniechu929 take a look.
| @@ -4,6 +4,7 @@ gemspec | |||
|
|
|||
| group :development, :test do | |||
| gem 'appraisal', '~> 2.1' | |||
| gem 'byebug' | |||
There was a problem hiding this comment.
I think this should be omitted. It feels like the tool(s) used for debugging comes down to the preference of the individual.
There was a problem hiding this comment.
Fine to remove it, just convenient to have when locally tracing code paths, and running the tests locally with bundle exec means it won't be available if it's not in the Gemfile. Stuff in the Gemfile doesn't get pulled into the gem release either, only relevant for someone developing locally.
|
Lemme know if there's anything I can do to help move this along, no worries if not, I'll just run our stuff against my branch for now. |
|
Thanks @mmrobins I've just chased this up internally as I'd prefer someone from our Ruby team to approve/merge. |
|
I addressed the comments in a followup commit. Lemme know anything else I can do to help get this merged and released |
|
@mmrobins I've released this now https://rubygems.org/gems/fastly/versions/3.0.2 |
|
Great, thanks! |
The class for SumologicLogging was declared, but it was never included
in fastly.rb to autogenerate the methods to call anything.
I added a test, but didn't manually update the comment documentation in
fastly.rb, as that looks hand generated and out of date for arguments,
so I figured the test was more useful.