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 spaghetti, or: restructure almost all the bare functions into struct members #6

Merged
merged 3 commits into from
Feb 11, 2022

Conversation

klardotsh
Copy link
Contributor

@klardotsh klardotsh commented Feb 11, 2022

Look, was this strictly necessary? probably not: technically things built as of #5. however, I had a litany of TODO comments in the source alluding to future refactors, and I finally got tired of seeing them, and also, in developing the CI commits in #5 I had to allow Clippy warnings in CI because of vestigial terrible decisions from the "hack and go" stage of development. let's get rid of those artifacts and allow Clippy to scream as loud as it so desires, eh? Singing is truly the way of life, after all.

  • Commit 1 is all regarding bundling user_script_registry_key with the actual Lua VM that generated it, rather than passing each as bare arguments all over the stack
  • Commit 2 is... everything else? I never hit a commitable intermediate state while going through the hundreds of type errors during this refactor, sorry. Basically, the step_* files now became non-step files that speak with Pipeline (a new struct) over strict interfaces, rather than the spaghetti I'd previously allowed. Thus, all StepHandlers now implement StepHandler and are semi-plugins to Pipeline ("semi" because they're still plumbed one-by-one and not loaded at runtime, but at least, they have extremely limited interfaces to speak over now).
  • Commit 3 is cranking up CI anger, since the warnings it previously complained about are now fixed, and when presented a linter, I can't help but put it on the strictest settings possible. Clippy warnings will now fail the build, as they should.

screenshot proof that UX has not regressed (plus, we have cargo test in CI with #5, which tests some units):
screenshot-2022-02-11-10:29:24Z

@klardotsh klardotsh requested a review from pdpol February 11, 2022 10:36
@klardotsh klardotsh force-pushed the josh/keto-aka-less-spaghetti branch 2 times, most recently from ec502db to 32c795e Compare February 11, 2022 19:01
Copy link
Contributor

@pdpol pdpol left a comment

Choose a reason for hiding this comment

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

Awesome restructuring!

Base automatically changed from josh/combinators-validators to init February 11, 2022 20:52
@klardotsh klardotsh force-pushed the josh/keto-aka-less-spaghetti branch from 32c795e to 9425d00 Compare February 11, 2022 20:53
@klardotsh klardotsh merged commit e969adf into init Feb 11, 2022
@klardotsh klardotsh deleted the josh/keto-aka-less-spaghetti branch February 11, 2022 20:57
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