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

Clean up DependencyMap #11

Open
djmitche opened this issue Dec 27, 2024 · 1 comment
Open

Clean up DependencyMap #11

djmitche opened this issue Dec 27, 2024 · 1 comment
Assignees
Labels
good first issue Good for newcomers
Milestone

Comments

@djmitche
Copy link
Collaborator

There are some TODOs in dependency_map.rs and in Replica::dependency_map. I think most are a no-op, but the error handling could be better.

@djmitche djmitche added this to the 1.0.0 milestone Dec 28, 2024
@djmitche djmitche moved this from Backlog to Ready in Taskwarrior Development Dec 28, 2024
@djmitche djmitche added the good first issue Good for newcomers label Dec 28, 2024
@djmitche djmitche moved this from Ready to In progress in Taskwarrior Development Jan 1, 2025
@djmitche djmitche self-assigned this Jan 1, 2025
djmitche added a commit to djmitche/taskchampion that referenced this issue Jan 1, 2025
Using Rc saves some atomic operations, at the cost of the result being
usable only from one thread. But lots of things are multi-threaded,
including Python, which causes issues trying to [wrap this
type](GothenburgBitFactory/taskchampion-py#11).
This isn't performance-critical code, so let's just us Arc.

This is a breaking change to the return type of
`Replica::dependency_map`.

This also adds a `Clone` derive to the type, just in case someone wants
the data without sharing.
djmitche added a commit to djmitche/taskchampion-py that referenced this issue Jan 1, 2025
The existing `Rc::into_inner` trick _never_ works, because the Replica
always holds one count of the `Rc` for itself, and returns a second
count -- so the reference count is always two. And, Rc::unwrap_or_clone`
doesn't work because `DependencyMap` can't be cloned.

This is a giant hack, as explained by comments inline. It's also
temporary, until
GothenburgBitFactory/taskchampion#514 is
released. I'll leave GothenburgBitFactory#11 open until that time.

Alternatives:
 - Parse the `std::fmt::Debug` output into a local data structure (!)
 - Just remove `Replica::dependency_map` from the API until
   taskchampion#514 is avaliable.
djmitche added a commit to GothenburgBitFactory/taskchampion that referenced this issue Jan 1, 2025
Using Rc saves some atomic operations, at the cost of the result being
usable only from one thread. But lots of things are multi-threaded,
including Python, which causes issues trying to [wrap this
type](GothenburgBitFactory/taskchampion-py#11).
This isn't performance-critical code, so let's just us Arc.

This is a breaking change to the return type of
`Replica::dependency_map`.

This also adds a `Clone` derive to the type, just in case someone wants
the data without sharing.
@djmitche djmitche moved this from In progress to In review in Taskwarrior Development Jan 1, 2025
@djmitche djmitche moved this from In review to In progress in Taskwarrior Development Jan 1, 2025
@djmitche
Copy link
Collaborator Author

djmitche commented Jan 1, 2025

Now waiting for GothenburgBitFactory/taskchampion#514, which will be in TaskChampion 2.0.0.

@djmitche djmitche moved this from In progress to Ready in Taskwarrior Development Jan 1, 2025
@djmitche djmitche moved this from Ready to Backlog in Taskwarrior Development Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Backlog
Development

No branches or pull requests

1 participant