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

Add more review rules 446 #466

Merged
merged 11 commits into from
Oct 31, 2020

Conversation

frankschmitt
Copy link
Contributor

Enables the remaining Unused review rules as discussed in #446. Things to note:

  • a couple of files have been excluded because they're reported, but are essential for the executable to work properly (false positives, see discussion for Add elm review for our elm code #446)
  • some minor changes to the copied external modules were necessary to make elm-review happy; if we decide to re-import them in the future, we'll need to re-apply these changes. This is not the optimal solution; we should probably either try to get the upstream modules to also use elm-review and clean up unused imports etc. or exclude them from elm-review.

Suggestions / feedback welcome :-)

@harrysarson harrysarson added the status: waiting for review Pull request needs a review label Oct 31, 2020
@harrysarson
Copy link
Collaborator

Thank you!

  • some minor changes to the copied external modules were necessary to make elm-review happy; if we decide to re-import them in the future, we'll need to re-apply these changes. This is not the optimal solution; we should probably either try to get the upstream modules to also use elm-review and clean up unused imports etc. or exclude them from elm-review.

Let's worry about this when it is time to update these modules

@harrysarson harrysarson merged commit 728c13a into rtfeldman:master Oct 31, 2020
@frankschmitt frankschmitt deleted the add-more-review-rules-446 branch November 1, 2020 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting for review Pull request needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants