remove getset from zingo-sync

[zancas]

No description provided.

[zancas]

Currently running CLT...

[Oscar-Pepper]

I am happy if you want to implement concrete getters (and settters where needed) to replace getset, this is valuable work that needs doing. however, i dont want to remove getset from the dependencies yet as i have alreafy mentioned, I am trying to move as quickly as possible so reducing boilerplate saves a lot of time when things are still is great flux during PoC, maybe you missed my reply about getset in PoC code, I hope you understand its uses in these cases. The reason why we want getters and setters instead of making things public is restricting mutability and also having a clear idea of where things are mutated is very important to avoid a whole class of bugs. If you have a look, the setters are used VERY sparingly, this was intended to avoid "update" bugs involved in race conditions etc.... the setters give us a clear idea of what is able to be modified and exactly where the changes happen.

[AloeareV]

I am happy if you want to implement concrete getters (and settters where needed) to replace getset, this is valuable work that needs doing. however, i dont want to remove getset from the dependencies yet as i have alreafy mentioned, I am trying to move as quickly as possible so reducing boilerplate saves a lot of time when things are still is great flux during PoC, maybe you missed my reply about getset in PoC code, I hope you understand its uses in these cases. The reason why we want getters and setters instead of making things public is restricting mutability and also having a clear idea of where things are mutated is very important to avoid a whole class of bugs. If you have a look, the setters are used VERY sparingly, this was intended to avoid "update" bugs involved in race conditions etc.... the setters give us a clear idea of what is able to be modified and exactly where the changes happen.

Well, we heard what you said about avoiding boilerplate and thought we'd remove getset and see how much boilerplate it necessitated adding, and discovered that it didn't seem to necessitate any as field access seemed more straight-forward.

The issue is that with field access it's harder to disambiguate reads from writes at a glance, or easily find all writes? A big part of the motivation to remove getset was to allow lsp goto-definition/goto-references to function.. The getset macro for me makes it harder to find these places than field access does, not easier, due to how much I rely on goto- lsp commands. What I'd really love would be if we could get the best of both worlds: Have a language-server ability to disambiguate mutable from immutable field accesses. Having getter and setter methods in cases where mutability isn't more private (which was only more private in iirc one field, at the moment) feels like unnecessary boilerplate even with the macros defining methods when we could just use field access.

Restricting mutability is a good use case, but I don't remember actually seeing any instances of it while doing this with @zancas in zingo-sync, I may have missed one...I remember there was one or two in zingolib proper. Maybe there will be more when it's no longer PoC? Even when we want to restrict mutability we likely want the greater-scoped mutability tucked behind a test-features flag, if we have primitives that can only be constructed correctly it can make it really hard to create 'junk' ones for unit testing our dependencies' code, but having different publicities on setters based on feature flags is yet another level of complexity. Maybe the way this library will be used this won't be a concern? I don't know how much the primitives here will be used, or if consumers will mostly just use the sync function.

All that said, I agree that having a clear idea of when things are mutated is great!

[zancas]

I am happy if you want to implement concrete getters (and settters where needed) to replace getset, this is valuable work that needs doing. however, i dont want to remove getset from the dependencies yet as i have alreafy mentioned, I am trying to move as quickly as possible so reducing boilerplate saves a lot of time when things are still is great flux during PoC, maybe you missed my reply about getset in PoC code, I hope you understand its uses in these cases. The reason why we want getters and setters instead of making things public is restricting mutability and also having a clear idea of where things are mutated is very important to avoid a whole class of bugs. If you have a look, the setters are used VERY sparingly, this was intended to avoid "update" bugs involved in race conditions etc.... the setters give us a clear idea of what is able to be modified and exactly where the changes happen.

I hear that there's a trade-off, with more awareness about the mutability vs. immutability of a field, but I am generally opposed to the (Pythonic) pattern of using hints in idents (e.g. _mut) to provide readability (even though I love Python). I think that information is available to the rustc and the developer and rustc should have the same understanding from the same source of truth.

A big positive of removing this library (besides the unambiguous positive of shrinking the dependency set, which is important), is that it allowed the LSP to work better by revealing dead code! This is critical ( I don't need to explain why.) so the positive intentions of getset actually inflicted a big negative by confusing standard tools, and breaking a critical feature of the language which is that the dev/tools know when private code is used.

I am strongly in favor of completely eliding getset, for this reason alone, but I also didn't find the putative benefit of removing boilerplate to actually exist! I think getters and setters (like ident hints) are pattern imports where the rationale is actually better handled by language features in Rust. Rust has great field access semantics.

[zancas]

@AloeareV the CLT has passed.

See above commentary.

[zancas]

Superceded by #1579