8000 Simplifying and making the os tag stringification maintainable by rigobert9 · Pull Request #4 · tristanisham/zvm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Simplifying and making the os tag stringification maintainable #4

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

Merged
merged 5 commits into from
Nov 24, 2022

Conversation

rigobert9
Copy link
Contributor

This commit contains some grammar checks (sorry for not squashing the commit, I wanted to keep those modifications separate), and replaces the exhaustive switch on the OS tag with a compile-time stringification of the enum (the value is replaced by its name, as it seemed intended).
I couldn't read the discord message that was linked in the comment, as it has likely been made unreachable by the conversion of the channel to a forum, I don't know if it needs to be removed.

Thanks for your hard work !

@rigobert9
Copy link
Contributor Author

Sorry for this one, I thought zig build ran the tests - haven't touched zig in a while. The tests passed after this small adjustment, my bad and sorry for taking your time with this kind of badly-formed PR.

Copy link
Owner
@tristanisham tristanisham left a comment

Choose a reason for hiding this comment

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

These changes look good! Thank you for taking the time to make optimizations and grammar fixes. There's always room for improvements and you've made some great ones!

After reviewing on my phone, everything looks good. I'll test the switch updates tomorrow along with other bug fixes before we release another alpha.

Have a fantastic Thanksgiving!

@tristanisham tristanisham merged commit a7abf9b into tristanisham:master Nov 24, 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.

2 participants
0