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 some misplaced parse warnings #4789 #4790

Merged
merged 2 commits into from
May 1, 2019

Conversation

snoyberg
Copy link
Contributor

This fixes the warnings about extraneous fields, by having PackageMetadata content parsed within the WarningParser which tracks which fields are used. This does not fully address #4789, please do not close that issue when merging this.

@snoyberg snoyberg requested a review from psibi April 30, 2019 19:49
@snoyberg
Copy link
Contributor Author

snoyberg commented Apr 30, 2019

Looking through this, it appears to me that we can simply remove the logWarn calls in Pantry.Archive which are generating the rest of the warnings, since lock files fully address this. Optionally, we could take a parameter to indicate whether we should warn on these, for use cases outside of Stack which don't have lock files, but that seems excessive. @qrilka what do you think?

EDIT This comment is probably wrong, ignore it. I'll put more info in the original issue when I have it.

Instead of FromJSON instances for Repo and PackageMetadata, use the
Data.Aeson.Extended mechanisms to properly track which fields are used.
@snoyberg snoyberg force-pushed the 4789-improved-from-json-instances branch from d4035f0 to 3f05443 Compare May 1, 2019 07:52
@snoyberg
Copy link
Contributor Author

snoyberg commented May 1, 2019

I've added an additional commit which addresses a different FromJSON instance parsing bug. I'm including it here since the test suite changes overlap with what is already present in this PR.

@snoyberg snoyberg force-pushed the 4789-improved-from-json-instances branch from 5ecab98 to bd647ff Compare May 1, 2019 09:19
@snoyberg snoyberg merged commit ba6037a into master May 1, 2019
@snoyberg snoyberg deleted the 4789-improved-from-json-instances branch May 1, 2019 14:14
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.

1 participant