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

Split GTReference into direct and symbolic variants #315

Open
jspahrsummers opened this issue Jan 17, 2014 · 5 comments
Open

Split GTReference into direct and symbolic variants #315

jspahrsummers opened this issue Jan 17, 2014 · 5 comments

Comments

@jspahrsummers
Copy link
Contributor

Some types (e.g., unresolvedTarget) and some methods (e.g., -referenceByUpdatingTarget…) change their behavior based on whether the receiver is a direct or symbolic reference.

We should clearly represent the differences in the type system, using an abstract GTReference superclass for the commonalities.

@tiennou
Copy link
Contributor

tiennou commented Apr 9, 2015

First, is this still wanted ?

I took a quick look, and I'm cautious about unexpected "class changes". For example, picture a Git client with an array of references that contains that one direct reference. Now I do some weird thing using the CLI, and that reference becomes symbolic. My Git client is now the proud owner of a clunky weird not-direct-but-direct nonetheless reference which will likely misbehave (this reminds me of #407 BTW).

Given the behavioral differences between direct/symbolic, I'm not sure it's worth to duplicate those 2 method just to enforce type-correctness (especially since that correctness can go out of the window anytime).

@jspahrsummers
Copy link
Contributor Author

We'd like to live in a world where libgit2 and ObjectiveGit objects are just immutable values, so changing the type of the reference on disk shouldn't affect its object representation anyways.

IMO, the behavior differences between direct and symbolic are the reason to have different types. They behave differently and mean different things, so they should look different too.

@tiennou
Copy link
Contributor

tiennou commented Apr 10, 2015

Yeah, I can see how trying to treat everything as immutable gives clear benefits for concurrency, but I don't get how this can work together with different types. I mean, what should we do if, as I explained above, one of those references' type changes below us ? Remap the class at runtime ? That seems opposed to the immutability we're aiming for...

The other subclass-y way would be to create the reference, stash its oid/target/name/whatever at initialization so we can be immutable, and call it a day. If the reference goes away from below, we can keep working, but it seems to duplicate things that libgit2 should handle (and maybe it does exactly that, and I'm just being clueless ;-)).

@jspahrsummers
Copy link
Contributor Author

The reference object can get out-of-date, but it should never become invalid due to an underlying change. We simply won't be watching for any such changes, so we won't know when they'd occur anyways.

If you wanted to manipulate a symbolic reference, but it got changed, I assume we would just generate an error. That's the sane thing to do no matter what, because it communicates to the caller that their expectations about behavior may be invalidated.

@mdiep
Copy link
Contributor

mdiep commented Apr 10, 2015

FWIW SwiftGit2 only exposes direct references. If it encounters a symbolic reference, it resolves it and returns the direct reference. That seemed to play the best with value semantics since you need to resolve to get the actual target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants