Kallithea issues archive

At this moment, the database model attaches comments to changesets or pullrequests (which are groups of changesets).

@kiilerix thoughts on this one?

Reviews of existing full files can pretty much be done with a PR to the null revision. But keeping track of the status per file is more tricky. The right datamodel for this would probably also imply partial reviews of files and keeping track of what has or hasn't been reviewed. It is also something that only will be needed once and probably have special needs.

I think I would suggest just start doing reviews of code changes and do cleanups as they are noticed. Once the review process for new changesets is running, start reviewing the existing files the manual and cumbersome way. Perhaps use something as lo-tech and flexible as a list of files in a spreadsheet.

Thank you for your help. Using a PR to the null revision may be sufficient to initiate a "clean review state" and begin use the review of changesets workflow. I will investigate this and report here.

I tried the following :

• adding a tag nullrev to rev 0 : hg tag -r 0 nullrev
• creating a pullrequest with origin nullrev and destination default

But the resulting pullrequest contains only the commit adding the tag nullrev. I didn't see a way of creating a pullrequest from a given revision.

@kiilerix could you precise what you add in mind with the PR to null revision ? Shall kalithea be modified to allow such a PR ?

I think @kiilerix meant creating a ‘detached’ revision:

hg up -r null
# … create files …
hg add .
hg ci -m "Added a bunch of meaningless files to review."

Thanks. Trying this leads to #149. I will investigate as soon as I get some time.

Mathieu: the PR should be the other way: from where the changesets are to where they are not yet. (It is inherently confusing that a "PR" conceptually goes in the opposite direction of a diff ...) (That method will however ignore revision 0 ... and it is not possible to put a tag on the null revision. Perhaps a bookmark or a localtag?)

I didn't mean to create a 'deteched' revision. Without a common ancestor it is really not possible to merge so i don't know what should happen ... but it shouldn't fail ...

I am using https://bitbucket.org/Unity-Technologies/kallithea/commits/78edbf29d5824d09c733f4b6a9fda70c9fea0a58#Lkallithea/controllers/compare.pyT244 which prevents this situation ... but I'm not entirely happy with it yet and haven't upstreamed it ...

A smaller fix that would work for this use case could be the following.

--- a/kallithea/controllers/pullrequests.py +++ b/kallithea/controllers/pullrequests.py @@ -337,8 +337,10 @@ class PullrequestsController(BaseRepoCon CompareController._get_changesets(org_repo.scm_instance.alias, other_repo.scm_instance, other_rev, # org and other "swapped" org_repo.scm_instance, org_rev, ) + if ancestor_rev is None: + ancestor_rev = org_repo.scm_instance.EMPTY_CHANGESET revisions = [cs.raw_id for cs in cs_ranges]

I will upstream it if you add a test case for it! ;-)

I tried, but was unable to run the testsuite.

I get the folowing error :

ConftestImportFailure: (local('/home/clabaut/contrib/kallithea/kallithea/tests/conftest.py'), (<class 'py._path.local.ImportMismatchError'>, ImportMismatchError('kallithea.tests.conftest', '/home/clabaut/contrib/kallithea/build/lib/kallithea/tests/conftest.py', local('/home/clabaut/contrib/kallithea/kallithea/tests/conftest.py')), <traceback object at 0x7fd0faa51200>))

I'll try to investigate, but in the meanwhile if you have already encountered and solved such an error, do not hesitate to tell me.

Can you clarify what exactly you did? Which command did you execute? Could you also send the output of 'pip freeze' ?

I also tried the following patch to allow creating a pull request with the null revision:

diff --git a/kallithea/controllers/pullrequests.py b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -153,10 +153,10 @@
         # prio 1: rev was selected as existing entry above

         # prio 2: create special entry for rev; rev _must_ be used
-        specials = []
+        specials = [('rev:0000000000:000000000000', '%s: %s' % (_("Changeset"), _("Null revision"))) ]
         if rev and selected is None:
             selected = 'rev:%s:%s' % (rev, rev)
-            specials = [(selected, '%s: %s' % (_("Changeset"), rev[:12]))]
+            specials.append((selected, '%s: %s' % (_("Changeset"), rev[:12])))

         # prio 3: most recent peer branch
         if peers and not selected:

But the PR shows 0 files changed with 0 insertions and 0 deletions even if it spans the whole repository history.

@patrickdepinguin I should have been more precise.

I ran py.test on the kallithea root directory.

Pip freeze gives:

Babel==1.3
Beaker==1.6.4
FormEncode==1.2.6
-e hg+*** failed to import extension hggit: No module named hggit
https://kallithea-scm.org/repos/kallithea@*** failed to import extension hggit: No module named hggit
1cdfd54a9c8ffbaccb4082677e3c494c638ce9b7#egg=Kallithea-dev
Mako==1.0.0
Markdown==2.2.1
MarkupSafe==0.23
Paste==1.7.5.1
PasteDeploy==1.5.2
PasteScript==1.7.5
Pygments==1.6
Pylons==1.0
Routes==1.13
SQLAlchemy==0.7.10
Tempita==0.5.3dev
UNKNOWN==0.0.0
URLObject==2.3.4
WebError==0.10.3
WebHelpers==1.3
WebOb==1.0.8
WebTest==1.4.3
Whoosh==2.5.7
amqplib==1.0.2
anyjson==0.3.3
celery==2.2.10
decorator==3.4.0
docutils==0.11
dulwich==0.9.9
kombu==1.5.1
mercurial==3.3.3
mock==1.0.1
nose==1.3.4
py==1.4.30
py-bcrypt==0.4
pycrypto==2.6.1
pyparsing==1.5.7
pytest==2.7.2
python-dateutil==1.5
pytz==2014.7
repoze.lru==0.6
simplejson==2.5.2
waitress==0.8.8
wsgiref==0.1.2

@matclab This looks like: https://bitbucket.org/pytest-dev/pytest/issues/539/pytest-doctest-modules-fails-if-python Is it possible you ran 'python setup.py build' at some point? I suggest removing that directory and running 'python setup.py develop' again, or (if that doesn't work) try again from a fresh setup.

Thanks @patrickdepinguin, it works now.

Please use the mailing list or IRC for discussions and keep each issue in the issue tracker "on topic" for discussion of one specific "flaw".

@Matclab What is the status of this issue? Were you able to add a test as suggested by @kiilerix in 144.html#comment-20523688 ?

I didn't have time to look at it for the last days… I hope to find some time in the two comming weeks…