Skip to content

Conversation

gomoripeti
Copy link
Contributor

@gomoripeti gomoripeti commented Mar 23, 2025

Keeping the function clause generic so that dialyzer does not complain
on older versions where the second arg of erlang:process_info/2 must be
an atom or list.

Keeping the function clause generic so that dialyzer does not complain
on older versions where the second arg of erlang:process_info must be
an atom or list.
Copy link
Owner

@ferd ferd left a comment

Choose a reason for hiding this comment

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

I don't think this is acceptable as-is just because we remove a whole check and change semantics without tests written. I think we should either keep existing clause but add a new one that checks for valid keys, or drop all checks but add tests.

@gomoripeti
Copy link
Contributor Author

thanks for the feedback. initially I had a strict matching on the dictionary tuple key (0c8269c) but dialyzer is complaining on older OTP versions.
I dared to remove the type guard because the info item names are not checked at all in recon if there are multiple in a list, just passed on to erlang:process_info/2. Because of this recon:info(Pid, [{dictionary, Key}]) already worked.

I will add tests.

@ferd
Copy link
Owner

ferd commented Apr 2, 2025

Yeah fair enough with Dialyzer. I imagine eventually it might make sense to deal with it like we do in other code base and only Dialyze newer versions (I need to bump up the CI used in this repo), but I'm good with removing the check for consistency with the test added on top.

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.

2 participants