search

so-dang-cool / dt

dt - duct tape for your unix pipes
https://dt.plumbing/
BSD 3-Clause "New" or "Revised" License
428 stars 12 forks source link

Say goodbye to Rust #29

Closed tensorush closed 1 year ago

tensorush commented 1 year ago

Let's politely show Rust where the exit door is. Also, fix minor typos and remove unused imports while we're at it.

UPD: Say goodbye to bash scripts as well 😉

booniepepper commented 1 year ago

Note: some build from source instructions will need to be updated

tensorush commented 1 year ago

Wow, I appreciate this so much!

Thanks also for the many typo corrections!

For import ordering, I don't have strong opinions. Are your changes there coming from something automatic, or just preference?

You're welcome! That's a cool project you've got here, so I decided to contribute a bit.

I tried to match your original style of imports with aliases grouped under their respective import + I usually keep grouped things sorted in ascending order, so I did that as well (there's a command for it in VS Code).

I hope in the future zig fmt will be enforcing a specific style of imports and removing unused ones. Perhaps, that's a good reason for a PR into the Zig compiler repo.

booniepepper commented 1 year ago

Jora, none of these are for you to do, just some rushed notes to myself to come back to. This PR was a good time for me to re-read a lot of things, like the tests, in depth.

UPD: Say goodbye to bash scripts as well :wink:

Some of these scripts were here because my test flow up to now was very REPL-centric. That was useful when it was a proof of concept but now it's a smell. I think Jora is right to just rip them out.

I need a place to write tests as fast as using a REPL. It was pointed out to me by @travisbhartwell that Porth from @tsoding has a pretty great setup (See: https://gitlab.com/tsoding/porth/-/tree/master/tests)

If I do re-introduce shell scripts (or dt scripts...?) then put them somewhere that isn't the root of the project. At least in a bin folder, or maybe a dev/bin and document them in a CONTRIBUTING.md

Really do open an issue to Zig on import ordering. Here there is churn in the PR that nobody really cared about. It'd be great to add this to zig fmt

Side note: Should I run zig fmt in CI/CD? As a githook? I've just been doing it manually

The project is mature and visible enough that it's worth writing tests on the individual builtin functions and solidifying them.

tensorush commented 1 year ago

Side note: Should I run zig fmt in CI/CD? As a githook? I've just been doing it manually

Yeah, I usually have a separate build step for it (like here) and then just run it in CI (like here).