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

refactor: make project more Zig-idiomatic #17

Merged
merged 3 commits into from
Jul 14, 2024
Merged

refactor: make project more Zig-idiomatic #17

merged 3 commits into from
Jul 14, 2024

Conversation

tensorush
Copy link
Contributor

@tensorush tensorush commented Jul 13, 2024

Hi @ATTron, great work on this library! It's cool to see more astro-/space-related Zig projects, especially since it's kind of been a Zig theme for some time, with the first Zig mascot, Zero, jet-packing around in a spacesuit.

I decided to contribute some changes to make your project have a more cleaned-up, Zig-idiomatic style.

See the official Zig Style Guide to explain some of the code changes. As for the removal of const Self = @This(); in most places, see this post from Loris.

Also, I have a project that generates a new Zig executable or library project with the common repo niceties included, some of which I've added in this PR. For instance, now you can run all the build steps at once with zig build. I wanted to add a step to run all the examples from the README as well, but I guess this PR is large enough, so I can do it for you in the next one.

Feel free to point out anything that you think should be done differently. Cheers!

@ATTron
Copy link
Owner

ATTron commented Jul 14, 2024

hey @tensorush! thanks for doing all this cleanup! Appreciate the help 🥇 Taking a look at this now, but so far it LGTM

@ATTron
Copy link
Owner

ATTron commented Jul 14, 2024

im happy with these. Thanks!

@ATTron ATTron merged commit 3b02296 into ATTron:main Jul 14, 2024
3 of 4 checks passed
@tensorush
Copy link
Contributor Author

Awesome! You might have to switch GitHub Pages deployment from "Deploying from a branch" (by default) to "GitHub Actions".
Screenshot

@ATTron
Copy link
Owner

ATTron commented Jul 14, 2024 via email

@tensorush
Copy link
Contributor Author

tensorush commented Jul 14, 2024

Yeah, must be a bug. Everything that is pub and has doc comments must be there. Maybe we should update the CD workflow to run on Zig master, since at 0.13.0 the new doc just got added, so it had more bugs. I can update those to master with the next PR if it's ok

@ATTron
Copy link
Owner

ATTron commented Jul 14, 2024

no worries, i can bump to master, and see how things look

@ATTron
Copy link
Owner

ATTron commented Jul 14, 2024

hmm yeah looks like that doesnt fix it. Oh well, no big deal for now, just whenever you have your next PR see if you can find a solution. Thanks!

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