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

Remove Arg module in favor of Weaver #204

Merged
merged 3 commits into from
May 24, 2024

Conversation

smores56
Copy link
Sponsor Contributor

@smores56 smores56 commented May 7, 2024

Zulip discussion

We have left the Arg module unmaintained except for the single Arg.list function. Otherwise, the parser in the Arg module is superceded by Weaver, so we can remove the built-in parser. By also moving Arg.list to Env.args, we can entirely remove the Arg module, as is done in this PR.

Copy link
Collaborator

@Anton-4 Anton-4 left a comment

Choose a reason for hiding this comment

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

Putting this on hold for discussion on zulip.

@smores56 smores56 force-pushed the remove-args-module branch 3 times, most recently from e77ed15 to 4bd9150 Compare May 12, 2024 20:45
@Anton-4
Copy link
Collaborator

Anton-4 commented May 13, 2024

Thanks for all the work you put into this @smores56!
This is quite a sizeable PR, I'm afraid I won't be able to review it this week because I'll be traveling.
I did put this PR on my TODO list to make sure I don't forget it!

@smores56
Copy link
Sponsor Contributor Author

This is quite a sizeable PR

Yes, it's quite large, so get to it when you get to it, it's not very high priority IMO since nothing blocked on it AFAIK. Thanks for the review when you do get to it!

@Anton-4 Anton-4 self-assigned this May 22, 2024
Copy link
Collaborator

@Anton-4 Anton-4 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 very much @smores56, this is some of the highest quality Roc code I have ever seen!

@Anton-4 Anton-4 merged commit 592a4a9 into roc-lang:main May 24, 2024
5 checks passed
@smores56 smores56 deleted the remove-args-module branch June 4, 2024 23:16
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.

None yet

2 participants