VirtusLab / git-machete

Probably the sharpest git repository organizer & rebase/merge workflow automation tool you've ever seen
MIT License
910 stars 52 forks source link

Consider switching to immutable collections for keeping the state (`managed_branches` etc.) #452

Open PawelLipski opened 2 years ago

PawelLipski commented 2 years ago

This has been done in https://github.com/VirtusLab/git-machete-intellij-plugin using vavr.io, with very good results. While CLI is less prone to race conditions on data structure as compared to GUI, it'd be still good to switch to immutable structures (to avoid the need for .copy() etc.).

As a first point, let's research what options for immutable structures are available in the ecosystem... esp. if anything is available in the standard library so that we don't need to add external deps 🙏🏻

amalota commented 2 years ago

I looked at the MacheteClient fields that keep state of the git machete and i see that the code very heavily depends on the state being mutable and i am not sure if its worth the effort and whether it would significantly improve the code quality. Could you maybe provide with some pros/cons or use cases besides the only one: for managed_branch in self.managed_branches.copy():.

Nevertheless, here are the possible solutions:

1. Tuple - immutable list

Pros:

Cons:

2. immutables - https://github.com/MagicStack/immutables

Pros:

Cons:

3. frozenset() - function that converts mutable iterable into immutable set

Pros:

Cons:

5. frozendict - immutable dict class - https://github.com/Marco-Sulla/python-frozendict

Cons:

6. Create our own immutable iterable classes

Pros:

Cons:

@PawelLipski wdyt?

PawelLipski commented 2 years ago

Hmmm okay, indeed lack of a single clear solution here... let's freeze this issue for some time, if serious problems (race conditions? but not very likely as almost everything is synchronous in this CLI) related to lack of immutability start occurring, we'll revisit