Skip to content

Conversation

@supersven
Copy link
Contributor

@supersven supersven commented Jan 25, 2022

Fixing these issues has two benefits:

  • The code is (hopefully) easier to read.
  • HLint issues that might indicate errors are easier to spot. (Less "noise" in HLint's output.)

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@supersven supersven force-pushed the sventennie/hlint-wire-api branch from a5f7c14 to 4dac022 Compare January 25, 2022 08:56
Copy link
Contributor

Choose a reason for hiding this comment

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

oops? why did you turn this on if it obviously compiled before?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, probably some overlap with RecordWildCards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, DisambiguateRecordFields is implied by RecordWildCards, but DisambiguateRecordFields is sufficient here.

Probably, this doesn't buy us much, but reducing HLint warnings helps to make the more important ones easier to spot...

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, probably some overlap with RecordWildCards?

Copy link
Contributor

Choose a reason for hiding this comment

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

@battermann and this is why you should not mix hlint changes and feature work in the same PR! :) (see #2035)

Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

Looks good.

There are a lot left, but these ones should be safe to not change the
application's behavior.
@supersven supersven force-pushed the sventennie/hlint-wire-api branch from 4dac022 to 1a99018 Compare January 26, 2022 07:38
@supersven supersven marked this pull request as ready for review January 26, 2022 08:42
@supersven supersven merged commit a915816 into develop Jan 26, 2022
@supersven supersven deleted the sventennie/hlint-wire-api branch January 26, 2022 08:44
@fisx fisx mentioned this pull request Jan 27, 2022
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.

4 participants