On 01/06/2017 08:07 PM, Andrew Shadura wrote:
# HG changeset patch
# User Andrew Shadura <[email protected]>
# Date 1474617925 -7200
# Fri Sep 23 10:05:25 2016 +0200
# Node ID d55296180cd477d84545d97d2abf1073ca8f0711
# Parent 1341be63734a5b90253bdd1589caf1a01e9266d1
git: add references for Git pull request heads
When a pull request is created, stick a reference to it in the refs/pull
namespace, similarly to what GitHub does, and remove it when the pull
request is deleted.
How likely is it that our use of refs/pull will conflict with other uses
of it? Are these names local to the repo managed by Kallithea and not
shared with clients?
That reference guarantees the commits stay around
and don't get garbage-collected when a PR is rebased or revised.
Also, that makes it possible to access head of the historis
(typo)
pull
requests using Git by fetching a ref from the source repository.
What does that mean?
Is it so that the pull command shown on a git PR also introduce a local
name for it so it is possible to check the revision out without getting
into detached head mode?
Unlike Git though,
But this is Git. Do you mean github?
we don't put the ref into the destination repository
and don't copy commits there to prevent undesired repository growth.
Kallithea uses global pull request identifiers, so there should not be
any confusion as to what pull request the ref corresponds to.
That would be because the PR id is a part of the ref? Perhaps clarify
the exact refs naming scheme first.
We may later provide a shim to redirect users to the source repository
if that deems needed.
diff --git a/kallithea/controllers/pullrequests.py
b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -330,6 +330,8 @@ class PullrequestsController(BaseRepoCon
if org_ref_type == 'rev':
cs = org_repo.scm_instance.get_changeset(org_rev)
org_ref = 'branch:%s:%s' % (cs.branch, cs.raw_id)
+ else:
+ cs = org_rev
In one branch cs is a changeset, in the other it is just the hash of it?
I suggest making it more consistent and just make the "cs =
org_repo.scm_instance.get_changeset(org_rev)" unconditional.
Or perhaps even better: just use org_rev instead of cs?
other_repo_name = _form['other_repo']
other_ref = _form['other_ref'] # will have symbolic name and head
revision
@@ -385,6 +387,11 @@ class PullrequestsController(BaseRepoCon
created_by, org_repo, org_ref, other_repo, other_ref,
revisions,
title, description, reviewer_ids)
Session().commit()
+
+ if org_repo.scm_instance.alias == 'git':
+ # create a ref under refs/pull/ so that commits don't get
garbage-collected
+ org_repo.scm_instance._repo["refs/pull/%d/head" %
pull_request.pull_request_id] = safe_str(cs)
+
h.flash(_('Successfully opened new pull request'),
category='success')
except UserInvalidException as u:
@@ -579,6 +586,14 @@ class PullrequestsController(BaseRepoCon
if pull_request.owner_id == c.authuser.user_id:
PullRequestModel().delete(pull_request)
Session().commit()
+
+ if org_repo.scm_instance.alias == 'git':
+ # remove a ref under refs/pull/ so that commits can be
garbage-collected
+ try:
+ del org_repo.scm_instance._repo["refs/pull/%s/head" %
safe_str(pull_request_id)]
The safe_str is a bit odd when we know it is a number. Perhaps use
pull_request.pull_request_id instead? It is perhaps a bit dirty to use
pull_request outside the transaction ... but the id must already be
fetched and I think it is better than safe_str.
I wonder if it is better to do this deletion inside or outside the
transaction ... but I can see how it is safer to do it outside.
+ except KeyError:
+ pass
+
h.flash(_('Successfully deleted pull request'),
category='success')
raise HTTPFound(location=url('my_pullrequests'))
@@ -803,8 +818,17 @@ class PullrequestsController(BaseRepoCon
h.HasRepoPermissionAny('repository.admin')(pull_request.org_repo.repo_name) or
h.HasRepoPermissionAny('repository.admin')(pull_request.other_repo.repo_name)
) and not pull_request.is_closed():
+ org_repo =
RepoModel()._get_repo(pull_request.org_repo.repo_name)
PullRequestModel().delete(pull_request)
Session().commit()
+
+ if org_repo.scm_instance.alias == 'git':
+ # remove a ref under refs/pull/ so that commits can be
garbage-collected
+ try:
+ del org_repo.scm_instance._repo["refs/pull/%s/head" %
safe_str(pull_request_id)]
+ except KeyError:
+ pass
This looks a bit like there should be a function for PR deletion so we
don't have to repeat this.
/Mads
+
h.flash(_('Successfully deleted pull request %s') %
pull_request_id,
category='success')
return {
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
