search

pact-foundation / pact-mock_service

Provides a mock service for use with Pact
https://pact.io
MIT License
73 stars 69 forks source link

Improve logging robustness #104

Closed lextiz closed 5 years ago

lextiz commented 5 years ago

Do not pretty print a JSON object when serialization or serialization fail, for details see #103.

lextiz commented 5 years ago

I am trying to do an end-to-end test, and build the executable according to instructions and have a question: which version of Ruby should be used, 2.2?

lextiz commented 5 years ago

Hi @bethesque,

I would really appreciate a hint regarding following the packaging instructions. After running the first step in the instructions (bundle exec rake package) with any recent Ruby version I get the following message:

You can only 'bundle install' using Ruby 2.2, because that's what Traveling Ruby uses.

When trying the same with Ruby 2.2 I get:

webrick-1.4.2 requires ruby version >= 2.3.0, which is incompatible with the current version, ruby 2.2.10p489
rake aborted!

Which version of Ruby should I use? Or am I missing something in the process?

Thanks!

lextiz commented 5 years ago

Tested this locally (found a way to do it without packaging the executable). IMO the PR is ready, please review.

bethesque commented 5 years ago

Firstly, please add a unit test to document this feature. No PRs will be accepted without test coverage.

Secondly, the original version of this method takes an object and returns a String. The change you have made means that it will return an object. Can you please explain this?

lextiz commented 5 years ago

Will have a look at the unit test. Regarding the return type: the purpose of this function is to pretty print a golang struct, when the pretty printing fails the object is returned and as far as I understand is accepted by the logger. What would you suggest instead?

bethesque commented 5 years ago

If the method is meant to return a string, then call object.to_s.

lextiz commented 5 years ago

@bethesque Thanks for the hint. IMO the code is ready for review.

lextiz commented 5 years ago

Is there something I can do in regards to this PR?