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

Fix mix task to pass pretty flag to custom codecs, add tests #676

Merged
merged 6 commits into from
Jan 27, 2019

Conversation

hadfieldn
Copy link
Contributor

This fixes an issue preventing the pretty flag from being passed to custom codecs. It also adds tests to ensure that CLI options are parsed correctly, appropriate defaults are used, and custom codecs are called successfully.

Note that in order to make the tests perform an actual conversion to JSON, we would have to add Jason or Poison as a test-mode dependency. I opted against this because if we can ensure that the codec is accessed correctly, the risk of Poison's or Jason's encode/2 method failing should be low.

However, if we don't add Jason or Poison as production-mode dependencies, we may want .to update the README to indicate that the application must load the the codec module in order for the mix task to work, even if using the default Jason encoder.

@hadfieldn
Copy link
Contributor Author

Just some context behind this -- I need this fix in order to support pretty formatting in a custom codec I wrote that generates schemas with types sorted alphabetically. This results in cleaner diffs when committing schema files to version control. See https://github.com/hadfieldn/absinthe_sorting_codec

Copy link
Contributor

@benwilson512 benwilson512 left a comment

Choose a reason for hiding this comment

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

Looks great! just a few notes.

lib/mix/tasks/absinthe.schema.json.ex Outdated Show resolved Hide resolved
lib/mix/tasks/absinthe.schema.json.ex Outdated Show resolved Hide resolved
lib/mix/tasks/absinthe.schema.json.ex Outdated Show resolved Hide resolved
hide public functions that are provided only for testing
@hadfieldn
Copy link
Contributor Author

Thanks for the helpful feedback @benwilson512. I implemented your suggestions and also cleaned up the docs a bit.

@hadfieldn
Copy link
Contributor Author

@benwilson512 Since this issue prevents the use of custom codecs in 1.4, would it be okay to create another PR that applies these changes to that branch?

@benwilson512
Copy link
Contributor

benwilson512 commented Jan 26, 2019 via email

@benwilson512
Copy link
Contributor

Thank you!

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