Skip to content

Diet of WORMs #143

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 6 commits into from
Dec 14, 2022
Merged

Diet of WORMs #143

merged 6 commits into from
Dec 14, 2022

Conversation

workingjubilee
Copy link
Contributor

This allows PL/Rust to feed on a filesystem that has been configured to only allow "write-once read-many" or WORM. It fixes #139 as one might imagine.

The unusual path remapping hack is only for this PR, I intend to fix it in the next. For various reasons I don't want to chance losing the git history for these files (yet).

This renames all State* FSM types to Fn*
- StateLoaded -> FnReady
- StateBuilt -> FnLoad
- StateValidated -> FnBuild
- StateProvisioned -> FnVerify
- StateGenerated -> FnCrating

This describes the state machine in present tense.
It previously used the past tense, which proved confusing.
This moves all the logic back to the start,
as the lib.rs is generated in a single step
so that it can be written only once.
This allows not losing git history via squashing
the commit that moves files along with this.
@eeeebbbbrrrr
Copy link
Contributor

Probably the only thing is I feel all the user_crate/*.rs are misnamed now that the state names have changed.

As an aside, I've never liked how some tests work. Comparing generated v/s expected rust code using syn/quote causes us to write that generated code multiple times. And it make reviewing a diff like this one incredibly difficult. Doesn't have to be part of this PR, but is there something we can do about this?

@workingjubilee
Copy link
Contributor Author

Yes, I want to do the git mv steps as something that gets merged as a separate commit so the history still follows.

Yeah, I made the tests use a more parametric form (partially generated, checking only the expected outline) to minimize that. I feel there's a lot of value in checking the overall outline against the fixture, and not as much in checking that every line is exactly the same, so I'm hoping the new form will tend to resist breaking on small changes.

@eeeebbbbrrrr
Copy link
Contributor

Yes, I want to do the git mv steps as something that gets merged as a separate commit so the history still follows.

Okay. I'll approve this now knowing that'll follow.

Yeah, I made the tests use a more parametric form (partially generated, checking only the expected outline) to minimize that. I feel there's a lot of value in checking the overall outline against the fixture, and not as much in checking that every line is exactly the same, so I'm hoping the new form will tend to resist breaking on small changes.

I'm not sure there is. These code checking tests only validate we typed the same sequence of characters in different parts of our code. They don't ensure the character sequences are correct as it relates to plrust's promises of "safety" and "trust".

I don't have a better idea at the moment, so just take that for what it's worth right now.

@eeeebbbbrrrr eeeebbbbrrrr self-requested a review December 14, 2022 19:13
@workingjubilee workingjubilee merged commit 338a97e into tcdi:main Dec 14, 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.

Oh, WORM?
2 participants