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

Remove go-spew dependency #483

Merged
merged 1 commit into from
Mar 11, 2021
Merged

Conversation

sapphire-janrain
Copy link

Removed the go-spew dependency since it's not really necessary. There were two places it was being used.

For parser_test I recreated the same form Sdump was outputting with pure fmt flags.

This was not possible for toml_testgen_support_test without reimplementing the functionality of go-spew, so I just did a basic, non-pretty dump. Personally, I think the non-pretty dump is better than prettying it up with json.MarshalIndent (which loses typing information but does not seem necessary). The reason for this is that it's easy to copy/redirect the terminal out to a file and visually compare the flattened lines to see when they stop being the same, this is much more difficult with pretty-printing as you have to jump back and forth etc. If you don't agree, I'm happy to change it to MarshalIndent.

For the original outputs vs the ones produced by the changes in this PR (and also the json.MarshalIndent version), see here.

@codecov
Copy link

codecov bot commented Mar 11, 2021

Codecov Report

Merging #483 (9af47ca) into master (6a307ac) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #483   +/-   ##
=======================================
  Coverage   92.84%   92.84%           
=======================================
  Files          12       12           
  Lines        2293     2293           
=======================================
  Hits         2129     2129           
  Misses        124      124           
  Partials       40       40           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a307ac...9af47ca. Read the comment docs.

Copy link
Owner

@pelletier pelletier left a comment

Choose a reason for hiding this comment

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

That's fair. Forgot it was still in there. Thank you!

@pelletier pelletier merged commit b59c12a into pelletier:master Mar 11, 2021
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.

2 participants