Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Fix locking #212
Conversation
zyga
added some commits
Dec 2, 2015
chipaca
reviewed
Dec 2, 2015
| @@ -81,6 +81,14 @@ func (r *Repository) HasType(t *Type) bool { | ||
| return false | ||
| } | ||
| +// HasType checks if the repository contains the given type. |
zyga
Dec 2, 2015
Contributor
Noted, I'll fix this in the next round. I have a few more Foo() == foo() + locking methods in the pipe
|
|
niemeyer
reviewed
Dec 2, 2015
| @@ -71,8 +71,8 @@ func (r *Repository) Add(cap *Capability) error { | ||
| return nil | ||
| } | ||
| -// HasType checks if the repository contains the given type. | ||
| -func (r *Repository) HasType(t *Type) bool { | ||
| +// hasType is identical to HasType but doesn't hold the repository lock |
niemeyer
Dec 2, 2015
Contributor
And you expect we, wannabe robots, to never mistype h by H or the opposite on every context? :-)
This should be named unlockedHasType or something. Or, alternatively, can we get rid of the public version altogether? I can't see many practical public use cases for a full-type comparison like this.
zyga
Dec 3, 2015
Contributor
+1 on unlockedFoo when there's a need for having both. I'll kill the public version in this case as you are right it is not really needed.
|
Still pending an answer to whether we can drop the public version, or whether we should rename it. |
zyga
added some commits
Dec 3, 2015
|
Sorry, I'm late to the party and now I'm confused. The branch says "adds locking" but I do not see locking anymore in the diff? Is github lying to me? Or did something else change here? |
|
The branch evolved to the point where loccking is gone as the only remaining method is private and you are expected to call it from public methods that already take the lock |
|
Thanks, in this case +1 from me |
zyga commentedDec 2, 2015
No description provided.