Skip to content

/git-lfs

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

Issue 3252: git-lfs locks should optionally denote own locks #3569

Merged
merged 5 commits into from Apr 16, 2019
+233 −95

Conversation

Reviewers

@ttaylorr ttaylorr

@bk2204 bk2204

@kh0kh0 kh0kh0

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@mstrap @bk2204 @ttaylorr @kh0kh0
@mstrap
Copy link

commented Mar 19, 2019

This is the same pull request as #3455 which I had closed unfortunately (by temporarily deleting my feature branch).

Marc Strapetz added some commits Jan 2, 2019

Marc Strapetz
locking: rename locks.VerifiableLocks to SearchLocksVerifiable 
SearchLocks and SearchLocksVerifiable are quite similar; also
API is using struct names like "lockVerifiableRequest".
f7c0a5c
Marc Strapetz
locking: SearchLocksVerifiable allows nil ref 
Refactor locks API to allow /locks/verify requests without a ref being
specified
f858772
Marc Strapetz
locking: SearchLocksVerifiable should use Client's RemoteRef 
Refactor SearchLocksVerifiable to use Client.RemoteRef instead of having
a separate ref parameter. This makes ref-processing more consistent with
other locks-functions.
b0e7a6f
@mstrap

This comment has been minimized.

Sign in to view
Copy link
Author

commented Mar 25, 2019

@git-lfs/core can you please take a look? It's the same pull request as #3455. The Travis CI build failure seems to be unrelated to my changes.
@bk2204

bk2204 reviewed Mar 26, 2019
View changes

Show resolved Hide resolved docs/man/git-lfs-locks.1.ronn Outdated
Show resolved Hide resolved locking/locks.go
Show resolved Hide resolved locking/locks.go Outdated
@mstrap

This comment has been minimized.

Sign in to view
Copy link
Author

commented Mar 27, 2019

@bk2204, thanks for reviewing. Do you prefer to have individual "fixup!" commits added or should I rewrite the feature branch by amending/squashing existing commits?
@bk2204

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Mar 27, 2019

@mstrap, you're welcome to do either. If you write individual fixup! commits, you'd have to squash before merging, but I'm okay with either. What I tend to do depends on how complex my branch is and how many fixups I plan to have; in this case, I'd probably just squash and amend.

Marc Strapetz added some commits Mar 28, 2019

Marc Strapetz
locking: new "locks --verify" option 
"locks --verify" verifies the server-side lock owner and denotes own
locks in the output by 'O'.

Internally, the /locks/verify API is used to have the server reporting
locks categorized by "ours" and "theirs". Note that this categorization
must be performed on the server because only the server knows the
mapping between login and lock owner name.

This basically resolves issue #3252 but does not yet address the
combination of "--verify" and "--cached".
ffe6220
Marc Strapetz
locking: "locks --verify" supports "--cached" 
"locks --verify" can be combined with "--cached" option now in the same
way as for "plain" remote queries.

Cached data is stored in a separate file "path/to/ref/verifiable", next
to the "plain" remote query cache file.

This resolves issue #3252.
Loading status checks…
ce1c1c0

@mstrap mstrap force-pushed the mstrap:issue-3252 branch from fc5a680 to ce1c1c0 Mar 28, 2019

@kh0kh0

kh0kh0 approved these changes Mar 29, 2019
View changes

@kh0kh0

kh0kh0 approved these changes Mar 29, 2019
View changes

@mstrap

This comment has been minimized.

Sign in to view
Copy link
Author

commented Apr 15, 2019

@ttaylorr may I ask you to give this PR a review, too? It complements the (remote) lock handling improvements which I had started with issue #3253.
@ttaylorr

ttaylorr reviewed Apr 16, 2019
View changes

Copy link
Member

left a comment

These changes seem reasonable to me, but I'll defer the final decision to @bk2204.
@bk2204

bk2204 approved these changes Apr 16, 2019
View changes

@bk2204 bk2204 merged commit 2bbe78f into git-lfs:master Apr 16, 2019
6 checks passed

6 checks passed

ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: build_with_earliest_supported_git Your tests passed on CircleCI!
Details
ci/circleci: build_with_latest_git Your tests passed on CircleCI!
Details
ci/circleci: build_with_system_git Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.