search

talentdeficit / jsx

an erlang application for consuming, producing and manipulating json. inspired by yajl
MIT License
695 stars 218 forks source link

Stricter dialyzer analysis #146

Closed paulo-ferraz-oliveira closed 4 years ago

paulo-ferraz-oliveira commented 4 years ago

Also [Fix #143].

okeuday commented 4 years ago

@paulo-ferraz-oliveira Thank you for making these changes! I tried them in the https://github.com/CloudI/CloudI repo and didn't see any problems.

paulo-ferraz-oliveira commented 4 years ago

@paulo-ferraz-oliveira Thank you for making these changes! I tried them in the https://github.com/CloudI/CloudI repo and didn't see any problems.

Cool, thank you. I did:

rebar3 dialyzer
rebar3 eunit

and, locally, these were without issues.

paulo-ferraz-oliveira commented 4 years ago

@talentdeficit, is this OK to merge?

tsloughter commented 4 years ago

Looks fine. Not sure why config type was removed though... hopefully not for a reason, maybe it was done on accident when I was merging some other changes recently.

paulo-ferraz-oliveira commented 4 years ago

Not sure why config type was removed though... hopefully not for a reason, maybe it was done on accident

Are you talking about type jsx_verify:config/0?

From what I could gather:

  1. it never existed in exported form (I parsed Git history, but may have done so incompletely),
  2. I removed it partially, in a previous PR you merged (https://github.com/talentdeficit/jsx/pull/123/files), since it was bugging my analysis, but the removal was incomplete, apparently.

Anyway, seems better now...

tsloughter commented 4 years ago

Ah ok if it never existed.