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

Add port_from_dynamic #68

Merged
merged 9 commits into from
Sep 3, 2024
Merged

Add port_from_dynamic #68

merged 9 commits into from
Sep 3, 2024

Conversation

rockerBOO
Copy link
Contributor

Replicates behavior similar to pid_from_dynamic for handling ports

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you. Could you add some tests and update the changelog please

@rockerBOO
Copy link
Contributor Author

Added tests and updated changelog

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you

@lpil
Copy link
Member

lpil commented Aug 16, 2024

Would you mind rebasing on main please 🙏

@rockerBOO
Copy link
Contributor Author

rebased

@rockerBOO
Copy link
Contributor Author

Maybe all the Port code should be in gleam_erlang because it's not really OTP related?

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you!

@lpil
Copy link
Member

lpil commented Aug 19, 2024

Maybe all the Port code should be in gleam_erlang because it's not really OTP related?

Just saw this after my review. You are totally right! We should move it there

@rockerBOO
Copy link
Contributor Author

Made a PR over there gleam-lang/erlang#55.

I think we should deprecate Port on here and use the one in gleam_erlang that I added. I think that is what confused me because I had already started work for the ports on gleam_erlang but they used the Port in OTP.

@lpil
Copy link
Member

lpil commented Aug 20, 2024

Good idea. Would you mind changing this to deprecate it with a message to use gleam_erlang? Thanks

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you!!

@lpil lpil merged commit 5f7a2ce into gleam-lang:main Sep 3, 2024
1 check passed
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