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 uint64 alias custom parser #52

Merged
merged 3 commits into from
Oct 20, 2018

Conversation

vith
Copy link
Contributor

@vith vith commented May 21, 2018

Needed to check for custom parsers before matching against basic types, since aliases of those types were matching in the switch, causing the custom parser to be ignored.

@ghost
Copy link

ghost commented May 21, 2018

There were the following issues with this Pull Request

  • Commit: f1eaf06
    • ✖ message may not be empty
      , - ✖ type may not be empty
  • Commit: 9f04fdc
    • ✖ message may not be empty
      , - ✖ type may not be empty
  • Commit: 0e21b77
    • ✖ message may not be empty
      , - ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@coveralls
Copy link

coveralls commented May 21, 2018

Coverage Status

Coverage increased (+0.004%) to 98.754% when pulling 299995a on vith:fix-uint64-alias-custom-parser into bc122c2 on caarlos0:master.

@vith vith force-pushed the fix-uint64-alias-custom-parser branch from 0e21b77 to 61c4a20 Compare May 21, 2018 15:31
@vith
Copy link
Contributor Author

vith commented May 21, 2018

The last commit I added, "remove redundant struct handling", was done because code coverage had shown only the error path of handleStruct was being reached now, so code coverage had been reduced.

There is a slight difference now, in that the custom parser for structs is looked up with funcMap[refType.Type] instead of funcMap[field.Type()]. All tests are still passing, but it is not clear to me whether these would always be identical. Is there another scenario I should add a test for where this would matter?

vith added a commit to kiwiirc/plugin-fileuploader that referenced this pull request May 21, 2018
@caarlos0
Copy link
Owner

Hi @vith ,

Sorry for the delay.

Can you rebase the work? It looks good :)

vith added 3 commits September 6, 2018 15:36
this was an issue for custom parsers registered for type aliases of
standard types, as the built-in parsers would match the type and get
used instead of the custom one.
…d for the underlying type or other aliases of that type
@vith vith force-pushed the fix-uint64-alias-custom-parser branch from 61c4a20 to 299995a Compare September 6, 2018 20:49
@vith
Copy link
Contributor Author

vith commented Sep 6, 2018

Sorry for the delay here too. It's rebased now. The CI failure looks like network flakiness I think? Maybe you can trigger a retry.

@caarlos0 caarlos0 merged commit 518fcfb into caarlos0:master Oct 20, 2018
@caarlos0
Copy link
Owner

thanks for the PR @vith !

nexoscp added a commit to nexoscp/env that referenced this pull request Apr 7, 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.

3 participants