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 a warning when there are only enums, no messages. #370

Merged
merged 2 commits into from
Dec 6, 2019
Merged

Conversation

judah
Copy link
Collaborator

@judah judah commented Dec 5, 2019

Previously we got:

...../build/enum_test/autogen/Proto/EnumOnly.hs:72:1: error: [-Wunused-top-binds, -Werror=unused-top-binds]
    Defined but not used: ‘packedFileDescriptor’
   |
72 | packedFileDescriptor
   | ^^^^^^^^^^^^^^^^^^^^

The check for whether the file had any messages was incorrect, since it didn't account for enums which are tracked in the same map.

Previously we got:

```
...../build/enum_test/autogen/Proto/EnumOnly.hs:72:1: error: [-Wunused-top-binds, -Werror=unused-top-binds]
    Defined but not used: ‘packedFileDescriptor’
   |
72 | packedFileDescriptor
   | ^^^^^^^^^^^^^^^^^^^^
```

The check for whether the file had any messages was incorrect, since it didn't account for enums which are tracked in the same map.
@judah judah requested a review from jinwoo December 5, 2019 20:51
@@ -163,3 +165,5 @@ testAliases = testCase "alias" $ do

testManyCases =
runTypedTest (roundTripTest "many cases" :: TypedTest ManyCasesProto)

testLone = testCase "no messages" $ LONE @?= LONE
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand this test. Isn't LONE always == LONE?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a comment. The test is intentionally trivial; the goal is just to make this file import the other module to make sure it compiles properly.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Thanks for the comment.

@@ -163,3 +165,5 @@ testAliases = testCase "alias" $ do

testManyCases =
runTypedTest (roundTripTest "many cases" :: TypedTest ManyCasesProto)

testLone = testCase "no messages" $ LONE @?= LONE
Copy link
Member

Choose a reason for hiding this comment

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

Got it. Thanks for the comment.

@judah judah merged commit 7bb31dc into master Dec 6, 2019
@judah judah deleted the no-warnings branch December 6, 2019 00:05
avdv pushed a commit to avdv/proto-lens that referenced this pull request Aug 9, 2023
Previously we got:

```
...../build/enum_test/autogen/Proto/EnumOnly.hs:72:1: error: [-Wunused-top-binds, -Werror=unused-top-binds]
    Defined but not used: ‘packedFileDescriptor’
   |
72 | packedFileDescriptor
   | ^^^^^^^^^^^^^^^^^^^^
```

The check for whether the file had any messages was incorrect, since it didn't account for enums which are tracked in the same map.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants