Re: merging approved pull requests
Hi Mads, On Wed, Dec 3, 2014 at 4:31 PM, Jan Heylen wrote: > Hi, > > we are trying to setup a POC Kallithea system to be able to do code review. > > what I did: > * setup a repo A > * fork rep A to A' > * clone A', make changes, push changes, amend changes, force push changes, > all to A' > * create a new pull request from A' to A > * approve the pull request, close the pull request > I have some additional questions regarding such review workflow: - is the described workflow a recommended one, in particular the need to force push changes? Or do you suggest something else? - when the developer modifies his changes based on review comments (using histedit or commit --amend for example) and issues a new pull request according to the described workflow, the old anonymous branch would remain on the A' repo. Currently there is no way to delete it, unless if an administrator goes into that repo and strips the changes manually. Is that correct? What about making it possible to delete associated commits when closing/deleting a pull request in Kallithea? - currently the creation of pull requests is an administrative action on the web interface. Are there plans to create a command-line helper / mercurial extension that handles the pushing and creation of the pull request towards Kallithea? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
kallithea documentation and references to version numbers
Hi, The current documentation, available at http://kallithea.readthedocs.org/en/latest/, sometimes contains references like [1]: - Starting from Kallithea version 1.2 ... - From version 1.4 Kallithea ... but these version numbers are not valid Kallithea references, but they are inherited from our ancestors. How should we handle such cases? What is the policy regarding mentioning our ancestors? My impression is that we're in a wizard's book referring to he-who-mustn't-be-named. Thanks, Thomas [1] http://kallithea.readthedocs.org/en/latest/api/api.html ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: kallithea documentation and references to version numbers
On Wed, Jan 14, 2015 at 2:18 AM, Mads Kiilerich wrote: > On 01/13/2015 09:04 PM, Thomas De Schampheleire wrote: >> >> Hi, >> >> The current documentation, available at >> http://kallithea.readthedocs.org/en/latest/, sometimes contains >> references like [1]: >> >> - Starting from Kallithea version 1.2 ... >> - From version 1.4 Kallithea ... >> >> but these version numbers are not valid Kallithea references, but they >> are inherited from our ancestors. >> >> How should we handle such cases? > > > Yes, that is wrong. > > I think we should enjoy that Kallithea is starting "now" and doesn't have > any history. We should remove such references to the past from the > documentation. We should however also make it more clear how to upgrade from > RhodeCode. > > The documentation could also benefit from more basic proof reading from > someone proficient in English/American. > > Other improvements of the content or structure of the documentation would > also be appreciated. But please keep iterations short so we avoid conflicts. Other problems are duplication/conflicts between the wiki and the documentation, for example there is a contributing section on both of them, but the contents are different: https://bitbucket.org/conservancy/kallithea/wiki/Home http://kallithea.readthedocs.org/en/latest/contributing.html We should decide one location for such information, and keep only that. Do you have a preference? The advantage of the in-tree documentation which is on readthedocs too, is that it is also available off-line for users that have the repo. But is readthedocs updated with the tip, or with the latest revision? ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #77: in-line review icon/button should also appear when hovering over the line number (conservancy/kallithea)
New issue 77: in-line review icon/button should also appear when hovering over the line number https://bitbucket.org/conservancy/kallithea/issue/77/in-line-review-icon-button-should-also Thomas De Schampheleire: When reviewing code, the green review icon/button that allows you to add comments for a specific code line only appears after hovering over the line, with a short timeout after that. If you hover over the line numbers, one would expect the same behavior but no review icon/button appears. This is not intuitive and should be fixed. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Tasks for the Next Release
Hi, On Sat, Jan 17, 2015 at 10:54 PM, Sean Farley wrote: > Prompted by Mads, I'm emailing the Kallithea list for discussion about > the next release :-) I was wondering what I would to see in the next > release and thought of the following: > > * finish font awesome (some minor bugs and missing logos) > * gravatar > * update hg compatibility to 3.3 (mainly for discovery improvement) > * documentation improvements!! (especially for installation) > > I'm working on my gravatar pull request this weekend but could use some > help with documentation. > > Any thoughts about this list? Of these items, I think documentation is the most important one. The first two are UI enhancements and not critical, and hg 3.3 compatibility is important but I would rank it below documentation. Regarding documentation, I would like to create a list of things to improve. Where would you suggest to place this: on a wiki page, or rather in a bitbucket issue? Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Tasks for the Next Release
Hi Mads, On Thu, Jan 22, 2015 at 2:33 PM, Mads Kiilerich wrote: > On 01/22/2015 01:09 PM, Thomas De Schampheleire wrote: >> >> Regarding documentation, I would like to create a list of things to >> improve. Where would you suggest to place this: on a wiki page, or >> rather in a bitbucket issue? > > > Kind of seriously: I think it would be much better if it was placed in a > pull request. Instead of creating a list, just start fixing something that > seems (most) important to fix. Nobody here can manage anybody else here and > tell them what to do, so such a list will be of limited value. > > That being said, feel free to put it in the issue tracker. Especially if you > can prove me wrong and show that it actually helps getting things done ;-) If I would have more time, I agree that just getting the work done, incrementally, would be the way to go. However, at this point, I have very limited time to spend, but I see ample places that could be improved in the documentation. So rather than keeping these ideas to myself, I wanted to list them so that others that do have time and care about documentation can implement some of them. An additional advantage of such a list is that it can help in reducing duplicate work, but maybe the risk is low in the case of documentation. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #79: pull requests: commit overview should preserve newlines in commit msgs (conservancy/kallithea)
New issue 79: pull requests: commit overview should preserve newlines in commit msgs https://bitbucket.org/conservancy/kallithea/issue/79/pull-requests-commit-overview-should Thomas De Schampheleire: On a pull request, in block 'Summary of Pull Request Content', the commit messages are not displayed verbatim. For example, blank lines are removed. I expect any formatting of the commit message (newlines, indentation) to be preserved. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #80: suggestion: indication of 80-chars limit in diff view (conservancy/kallithea)
New issue 80: suggestion: indication of 80-chars limit in diff view https://bitbucket.org/conservancy/kallithea/issue/80/suggestion-indication-of-80-chars-limit-in Thomas De Schampheleire: In diff views, it would be nice to be able to see where the 80 / 120 / ... character limit is, mainly useful for code review in projects where there is a limit on line length. Some example implementations: - a kind of ruler at the top of each diff section (for each file) where column 10, 20, ... is indicated. - similar to vim's colorcolumn, a permanent different background color of column 80, 120, ... - an indication on the line itself, only visible when hovering the line The columns to highlight could be fixed, a configuration option (per project, or per user), or dynamically changeable at the top of the diff view (with a default value). ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #81: diff view: copy code improvements (conservancy/kallithea)
New issue 81: diff view: copy code improvements https://bitbucket.org/conservancy/kallithea/issue/81/diff-view-copy-code-improvements Thomas De Schampheleire: Copying code from a diff view is somewhat odd: - when selecting lines, the line numbers are part of the selection - when pasting, the line numbers are not present (which is a good thing in fact) - but blank lines are introduced in between each line It would be nicer if: - during selection, only the code is selected - pasting does not insert blank lines ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #82: code comments: ability to edit comments (conservancy/kallithea)
New issue 82: code comments: ability to edit comments https://bitbucket.org/conservancy/kallithea/issue/82/code-comments-ability-to-edit-comments Thomas De Schampheleire: Currently, an existing comment can be deleted only, not edited. Scenario: - user writes a big comment using RST syntax and submits - user then realizes a mistake, or forgot to add a review status (approved/rejected/...) Rewriting the RST syntax is annoying, there is no way of getting the source of the original comment, and adding an extra comment to indicate the mistake/set status is not very nice. Editing should only be possible by the original author of the comment. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #83: side by side diff: missing code when scrolling (conservancy/kallithea)
New issue 83: side by side diff: missing code when scrolling https://bitbucket.org/conservancy/kallithea/issue/83/side-by-side-diff-missing-code-when Thomas De Schampheleire: In a side by side diff of a file that is longer than screen height, when scrolling on one side, the code lines on the other side stop at some point, showing a blank area. When you move your mouse to the other side and scroll there, the missing line suddenly reappear. This behavior also happens when scrolling from 'the other side'. Seen on Linux, Firefox 33.0 (not tested on other platforms/browsers). ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #84: compare revisions: in-line comment icon appears but is not clickable (conservancy/kallithea)
New issue 84: compare revisions: in-line comment icon appears but is not clickable https://bitbucket.org/conservancy/kallithea/issue/84/compare-revisions-in-line-comment-icon Thomas De Schampheleire: When comparing two revisions and hovering a code line, the comment icon appears but nothing happens when you click it. Either we should not show the icon, and thus disallow comments from a compare view, or the icon should be clickable and a comment box should appear. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
new commits to Kallithea should be sent to the mailing list
Hi, Currently, new commits on the Kallithea project silently end up in Bitbucket. In order for users to become aware of such changes and to create an active community, I believe a mail notification of these commits should be sent to the mailing list. I would prefer one mail per commit, but an alternate style is how Mercurial does it: one mail per bunch of commits. What do you think? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
discussion on issue #77 (review icon position for in-line code comments)
Hi, This mail thread is a continued discussion of issue #77 (in-line review icon/button should also appear when hovering over the line number) [1] The problem statement: if I place my mouse at the location where the review icon normally appears, I expect it to appear. Currently, this is not the case, as the icon is outside of the area that triggers the icon to appear. My requirement is thus: a. the area containing the icon should be a hotspot that causes the icon to appear. Mads already hinted on some other requirements: b. we don't want the icon to overlap with the code c. the clickable area should be larger than the icon itself to make it more easy to hit it. d. we don't want to reserve additional dedicated space e. we want to be able to link to a given code line Your proposals are very welcome. Thanks, Thomas [1] https://bitbucket.org/conservancy/kallithea/issue/77/in-line-review-icon-button-should-also ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #91: code comments: links to 'next comment' should cross inline/commit boundary (conservancy/kallithea)
New issue 91: code comments: links to 'next comment' should cross inline/commit boundary https://bitbucket.org/conservancy/kallithea/issue/91/code-comments-links-to-next-comment-should Thomas De Schampheleire: Commit views contains at the top: '4 comments (2 inline) First comment'. The 'First comment' link points to the first inline comment. At the top of this first inline comment, a 'Next comment' link to the second inline comment is provided. However, there is no 'Next comment' from that last inline comment to the first general comment. This should be added, so that there is one chain to follow to see all comments on one commit. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
commit fab4e18432bae0f11a9ed3d5c682c2442d2ef007 breaks pull request updates
Hi Mads,
I just updated to the latest tip of kallithea, and now see that
updating a pull request with a new descendant does not work.
Kallithea detects there is a new descendant (text 'This pull request
can be updated with changes on default:') is shown, but no commits are
shown.
In commit fab4e18432bae0f11a9ed3d5c682c2442d2ef007, you added:
show = set(org_scm_instance._repo.revs('::%ld & !::%s & !::%s',
avail_revs, revs[0], c.a_branch_name))
but the c.a_branch_name is 'default' in my case, while the avail_revs
are also on default. This causes anything on the default branch to be
ignored.
If I remove the last '& !::default' clause, I get the expected list.
Could you clarify the reason for this extra clause, what branch you
expect there?
I assume that just removing the clause does not match your own use case?
Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: save multiple comments at once
On Tue, Jan 27, 2015 at 4:04 PM, Andrew Shadura wrote: > On 27 January 2015 at 14:11, Nick Coghlan wrote: >> On 27 January 2015 at 04:35, Mads Kiilerich wrote: >>> I don't see the point in rst/md markup of comments. People should comment on >>> content, not spend time making it look fancy >> >> In Gerrit, there are two specific aspects of MD rendering I regularly >> find useful: monospace formatting of indented code snippets, and >> bullet point formatting of bulleted lists. >> >> So those aren't about being fancy, they're about making the comments >> easier to read. > > I agree. Md/RST is something that's generally useful and improves the > readability. It should probably be a per-instance or per-repository > setting, I guess. > It could be a setting, but for the features Nick mentions I don't see much added value: if the entire comment would be monospaced, then code snippets will too. And bulleted lists are equally readable with plain *, no ? ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: commit fab4e18432bae0f11a9ed3d5c682c2442d2ef007 breaks pull request updates
Hi Mads,
On Tue, Jan 27, 2015 at 10:55 PM, Mads Kiilerich wrote:
> On 01/27/2015 04:04 PM, Thomas De Schampheleire wrote:
>>
>> Hi Mads,
>>
>> I just updated to the latest tip of kallithea, and now see that
>> updating a pull request with a new descendant does not work.
>> Kallithea detects there is a new descendant (text 'This pull request
>> can be updated with changes on default:') is shown, but no commits are
>> shown.
>>
>> In commit fab4e18432bae0f11a9ed3d5c682c2442d2ef007, you added:
>>
>> show = set(org_scm_instance._repo.revs('::%ld & !::%s & !::%s',
>> avail_revs, revs[0], c.a_branch_name))
>>
>> but the c.a_branch_name is 'default' in my case, while the avail_revs
>> are also on default. This causes anything on the default branch to be
>> ignored.
>>
>> If I remove the last '& !::default' clause, I get the expected list.
>>
>> Could you clarify the reason for this extra clause, what branch you
>> expect there?
>> I assume that just removing the clause does not match your own use case?
>
>
> I'm sorry about that. Yes, it works just fine for our use where we abuse
> named branches for feature branches ;-)
>
> I guess your case is that you have server side forks and don't use named
> branches?
Correct.
>
> That point with this change is that a PR is a preview of a merge. The
> preview is a diff from the common ancestor. But when it is merged, it will
> usually not be merged to the ancestor but to the head of the target branch.
> It is thus not relevant to update to or show anything that already is an
> ancestor of the target branch.
>
> Will
> https://bitbucket.org/Unity-Technologies/kallithea/commits/6cb95b08cc0b27a61b5df0cc0cec145e8e987e9c
> fix your case?
>
Yes, I tested and it does. Please push the fix to kallithea central.
> I can promise that your use case will keep working in the future if you add
> test coverage for it ;-)
Is there any coverage already related to pull requests and possibly
pull request updates? I don't see any.
It would indeed be much better to have all this tested, but without a
good example I'm a bit at lost here.
Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: commit fab4e18432bae0f11a9ed3d5c682c2442d2ef007 breaks pull request updates
On Wed, Jan 28, 2015 at 1:15 PM, Mads Kiilerich wrote: > On 01/28/2015 12:12 PM, Thomas De Schampheleire wrote: >> >> Hi Mads, >> >> >>> I can promise that your use case will keep working in the future if you >>> add >>> test coverage for it ;-) >> >> Is there any coverage already related to pull requests and possibly >> pull request updates? I don't see any. >> It would indeed be much better to have all this tested, but without a >> good example I'm a bit at lost here. > > > I think you are right that there not is any test coverage of this area. > > The existing tests use a unittest framework but it is not unittests at all. > That makes it a bit painful to maintain. Still, it is better than nothing > ... if we have tests. > > I guess a test would have to build a hg repo with the right structure, > invoke Kallithea, and verify that the html output has the expected content. In fact, I think it would be better if this could be tested without having to verify html output. The test would create a repo with the right structure, create a pull request using regular python code (I mean, without sending http requests or the like) update the repo, check the list of revisions available for update. To put it differently: I think we should (in so much this is not already the case; I haven't looked at all tests) decouple the testing of the backend (the real code handling) from the frontend (html, js, css, http, ajax, ...). This may need some code restructuring though. For example, PullrequestsController.show(), the method where this problem is located, is one big method. It is not possible to read out the list of revisions that are available for update from regular Python code, so this is not testable. Instead, a helper method should be created that returns the list of revisions available to update a given pull request, which can then be called by the test. > > There is also > https://bitbucket.org/conservancy/kallithea/pull-request/33/add-first-robotframework-pullrequest-test > . It is even more high level and more "realistic" but probably also slower > ... and it doesn't work when I try to use it. As far as I can see from the example test, this is testing from the user perspective, like a robot indeed. But as I said above, I think we should decouple the backend from the frontend testing. The robot framework can be valuable to test some aspects of the frontend part, but assuming there is good test coverage on the backend stuff, it doesn't make sense to cover all scenarios in the robot framework. Moreover, again based on the example tests, the test method is pretty fragile, as it relies on certain html/css object names that could easily change. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #93: pull requests: link from commits back to pull request (conservancy/kallithea)
New issue 93: pull requests: link from commits back to pull request https://bitbucket.org/conservancy/kallithea/issue/93/pull-requests-link-from-commits-back-to Thomas De Schampheleire: When browsing the commits in a pull request, there is no direct way to go back to the pull request. The back button of your browser only works if you did not clicked into deeper commits using the 'parent rev' / 'child rev' links. In general, there is no connection between commits and the pull requests they are part of, nor is there a connection (AFAICS) between comments on a commit and the pull request it was made in. I think such a connection could prove useful. At a minimum, we should add a link from commits to their containing pull request. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Code review: pull requests versus independent review
Hi, Currently, reviewing a set of commits in Kallithea is based on the concept of a pull request. A code author can create a pull request for a set of (descendant) commits so that people can review it. When Kallithea is not used for actually performing the pull/push to the main repo, the pull request concept is actually not the best fit. One particular case where it fails is when not all commits you want reviewed are descendants of one another. Maybe there is a commit in the middle that is irrelevant, or relevant but you do not expect reviewed by that set of people. In this case, being able to cherry-pick the commits part of the review is more useful. Given the Kallithea way of handling reviews (which is mostly coupled to individual changesets without them being coupled to the corresponding pull request), it wouldn't be that hard to add an extra concept in Kallithea: that of 'Review request'. Such review request could contain an arbitrary number and combination of commits, without mandatory parent-child relation, but is otherwise very similar to the current pull request concept. What do you think of this idea? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: unwanted auto status update of commits on pull request update
On Mon, Jan 26, 2015 at 6:18 PM, Jan Heylen wrote: > So can I also conclude that this is to be considered a bug? > > I'll try to dig into it, if you can be a bit more specific what to look for > ;-) > > On Mon, Jan 26, 2015 at 6:07 PM, Mads Kiilerich wrote: >> On 01/26/2015 06:01 PM, Jan Heylen wrote: >>> >>> Hi, >>> >>> I noticed following behavior: >>> When updating a pull request (adding a new commit), without changing the >>> other commits in the pull request, Kallithea automatically updates the >>> status of the original commits, back to 'under review'. >> >> >> It is a consequence of the broken data / database model. Try to dig into it. >> Lots of fun there ;-) Possibly related to this issue, I notice that the comment count as visible on the commit overview of the original pull request overview page (after having created an update) corresponds to the comment count of the new pull request commits (even for these commits that did not change). When you open such commit, the comment count is correct. For example: - v1 of the review had 3 comments on a particular commit. The overview of v1 shows '3' next to this commit. - v2 is created. The overview of v2 now shows '1' next to the commit, caused by the 'Auto status change to Under Review' comment. - however, v1 now *also* shows '1' next to that commit, instead of '3'. In fact, correct behavior (assuming we like the preserve-comments-on-the-same-commits), would be to show '3' in both v1 and v2. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Code review: pull requests versus independent review
Hi Nick, On Wed, Jan 28, 2015 at 2:50 PM, Nick Coghlan wrote: > On 28 January 2015 at 23:06, Thomas De Schampheleire > wrote: >> Hi, >> >> Currently, reviewing a set of commits in Kallithea is based on the >> concept of a pull request. A code author can create a pull request for >> a set of (descendant) commits so that people can review it. >> When Kallithea is not used for actually performing the pull/push to >> the main repo, the pull request concept is actually not the best fit. >> >> One particular case where it fails is when not all commits you want >> reviewed are descendants of one another. Maybe there is a commit in >> the middle that is irrelevant, or relevant but you do not expect >> reviewed by that set of people. >> >> In this case, being able to cherry-pick the commits part of the review >> is more useful. Given the Kallithea way of handling reviews (which is >> mostly coupled to individual changesets without them being coupled to >> the corresponding pull request), it wouldn't be that hard to add an >> extra concept in Kallithea: that of 'Review request'. >> Such review request could contain an arbitrary number and combination >> of commits, without mandatory parent-child relation, but is otherwise >> very similar to the current pull request concept. >> >> What do you think of this idea? > > What would be the expected outcome of such a review request, and how > do you see it fitting into a team's workflow? The outcome is that the developer gets a go/no-go feedback for his changes, giving him the permission to proceed with delivering his changes through the existing methods (which could be outside of Kallithea). If comments were posted, the author is supposed to modify his code and relaunch/update the review request. The workflow would be a. create changes in local repo b. push changes to review repo in kallithea c. launch review request in kallithea d. receive comments from reviewers e. adapt code, go to b. (cycle) f. when approved, push code to delivery repo (outside of kallithea, possibly passing some validation). > > With a pull request, the expected outcome is clear (merged or > rejected), and if fits clearly into a workflow as a gating mechanism > for inclusion into the main line of development. Could you explain in some detail how Kallithea can be used to support this workflow, in particular assuming there are no comments on the pull request, how does the code get merged? I did not see any provisioning in Kallithea to merge in the code, so the developer is supposed to pull the changes from the pull request, perform any needed merge, then push the changes back. Is that correct? And (how) does Kallithea detect that the changes are pulled? Is it possible to indicate that a pull request is merged, albeit after rebasing? > > Additionaly, how would such a review request differ from cherry > picking the commits of interest to a new branch and including them in > a new pull request? The fact that you need to do the cherry-picking to the branch. With the review request, the repository is not touched, only viewed in a different way. When cherry picking, you are effectively changing the repository, in order to fit it into the limitations of a tool. Note that for my own changes, a pull request would probably suffice, but I'm thinking in a large company context where there are many developers, some of which may not even know how to cherry-pick. > > Finally, if Kallithea is not the "repo of record" and someone just > wants a standalone code review tool, wouldn't it make more sense to > use something like ReviewBoard instead? We did evaluate several other tools, including ReviewBoard, Crucible, Phabricator and Upsource. But only Kallithea and Upsource meet my basic requirement of being able to review each commit separately in case of a multi-commit review. Note also that although at this moment we are still using mercurial-server to host the repositories, I do not exclude that Kallithea could take over this job once SSH authentication is supported. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Kallithea internals overview
Hi, What would be useful to me (and probably to other new contributors) is a technical overview of Kallithea's internals. Browsing the code as a newcomer only provides so much insight; it's difficult to get a good overview of the different components and how they fit together. For example, there are - templates - controllers - models - ... ? and then there are technologies like pylons, cellery, rabbitmq, fontello ... and various javascript libraries like mergely, select2, jquery and what not. Even though I already know for some of them how they (more or less) fit in the picture, a grand overview of all this would be a big help. This could even be added in the documentation. Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Kallithea internals overview
On Thu, Jan 29, 2015 at 1:00 AM, Mads Kiilerich wrote: > On 01/28/2015 08:14 PM, Thomas De Schampheleire wrote: >> >> Hi, >> >> What would be useful to me (and probably to other new contributors) is >> a technical overview of Kallithea's internals. >> >> Browsing the code as a newcomer only provides so much insight; it's >> difficult to get a good overview of the different components and how >> they fit together. >> >> For example, there are >> - templates >> - controllers >> - models >> - ... ? >> >> and then there are technologies like pylons, cellery, rabbitmq, >> fontello ... and various javascript libraries like mergely, select2, >> jquery and what not. >> >> Even though I already know for some of them how they (more or less) >> fit in the picture, a grand overview of all this would be a big help. >> This could even be added in the documentation. > > > If you make a first draft with the things you already have figured out and > the qualified guesses you have made, then I will make a similar iteration > and fix/add what I might be able to add on top of that. Let me give this a try. It's not very structured, but it could become better when the picture is more complete. Frontend: - URLs are connected to actual code using pyRoutes, configured at config/routing.py. The code that is called is part of controllers, but how to define/describe a controller? Templates (kallithea/templates) define the core of the user interface. They intermix regular HTML with Javascript code (plain Javascript or frameworks like jQuery, YUI, (others?)). The templates are parsed using the 'mako' template library. Data regarding commits/repositories/... is loaded dynamically from the server through AJAX calls. Question: what is the reasoning to both use jQuery and YUI? Can't we select one framework and do everything in that? Answer: according to the wiki 'future' list, YUI should be killed in favor of jQuery. Related question: a bird told me that it may be better to use a higher-level framework like AngularJS instead of directly scripting stuff either in plain Javascript or jQuery. Has there been any thought about this before? On the 'future' list, I see "move code from templates to controllers and from controllers to libs or models". Can you clarify this a bit more? Question: how is the relation between the REST API (controllers/api) and the AJAX calls? Is the API used internally in some way, or only provided for external usage? The codemirror JS library (http://codemirror.net/) is used to display code files. The mergely JS library (http://mergely.com) is used to display diffs of code. Mergely is based on codemirror. fontello is a library/service to create a font of symbols (instead of using icons in image files). Did we create our own font or are we using an existing one? 'font awesome' is a specific symbol font. How does it relate to fontello? pygments: syntax highlighting formencode: form validation excanvas? mousetrap? native.history.js? Backend: Revision information is obtained from the VCS library (lib/vcs), which in case of Mercurial interacts directly with the Mercurial python classes, and in case of git uses the dulwich python module. models: this is a class representation of the database tables. Which framework/component handles the relation between these classes and the actual database? I guess using SQLAlchemy? How does this relate to Django? pylons: web framework, but what exactly does it do? On the 'future' list, I see: 'more best practice for web apps and the frameworks', can you clarify a bit more? paste: ? bcrypt: password hashing celery: distributed task queue (how is this used?) whoosh: code indexing/search Question: what is all this WSGI stuff? If you start Kallithea according to the base instructions, it's hosting it's own web interface (without WSGI?) How does WSGI work, what are the advantages/disadvantages? Related to this, I see people running Kallithea under Apache, advantages/disadvantages? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
pull requests: default target branch
Hi, When creating a pull request in Kallithea, the tip (or specified revision) in the originating repository is compared with the destination repo's tip. When having several release branches, this choice could be confusing: suppose someone makes changes on the default branch and creates a pull request. If the last change in the target repo happens to be on the branch of another release, the list of revisions is huge. The creator of the pull request may not immediately realize that he should select the default branch explicitly. There already exists the concept of 'landing revision' but it does not seem to be used here. A proposal is: - for the source, use the current tip (this is typically the change of the developer) - for the target, use the landing revision (which could be changed to the default branch in my case). An alternative is: - for the source, use the current tip (this is typically the change of the developer) - for the target, use the same branch as the source is on, if it exists, otherwise use the landing revision (or the current tip). Other ideas? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #94: user administration improvements (conservancy/kallithea)
New issue 94: user administration improvements https://bitbucket.org/conservancy/kallithea/issue/94/user-administration-improvements Thomas De Schampheleire: - e-mail sent to the administrator when a new user signs up should clearly state that his action is required to activate the account - a link to the activation page should be present - the web interface would ideally contain an easy way to activate one or multiple accounts, ideally by clicking one button and not requiring to 'edit' each account individually. A 'activate all non-activated users' button could also help here. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Kallithea internals overview
Hi, On Thu, Jan 29, 2015 at 11:50 AM, Thomas De Schampheleire wrote: ... >>> What would be useful to me (and probably to other new contributors) is >>> a technical overview of Kallithea's internals. Since my first attempt, I've been reading documentation of pylons and mako, which was very enlightening to understanding Kallithea. http://docs.pylonsproject.org/projects/pylons-webframework/en/latest/ http://docs.makotemplates.org/en/latest/ Below I am updating parts of this overview with the gathered knowledge. I removed the things that are no longer relevant or accurate. Framework: -- The pylons framework forms the core of the Kallithea application. It defines a lot more than I had imagined, including the directory tree and the set of additional Python packages being used. The concepts models, views, controllers also stem from this framework. When creating a pylons application, the skeleton structure is: ├── config │ ├── deployment.ini_tmpl │ ├── environment.py │ ├── __init__.py │ ├── middleware.py │ └── routing.py ├── controllers │ ├── error.py │ └── __init__.py ├── __init__.py ├── lib │ ├── app_globals.py │ ├── base.py │ ├── helpers.py │ └── __init__.py ├── model │ └── __init__.py ├── public │ ├── bg.png │ ├── favicon.ico │ ├── index.html │ └── pylons-logo.gif ├── templates ├── tests │ ├── functional │ │ └── __init__.py │ ├── __init__.py │ └── test_models.py └── websetup.py Which is precisely what Kallithea has for structure. See http://docs.pylonsproject.org/projects/pylons-webframework/en/latest/gettingstarted.html#creating-a-pylons-project for details. The principle is as follows: - URL paths are coupled to 'controllers' through the routes packages. A controller is basically a python class with some methods. Based on these routes, a specific method of a certain controller will be called to handle a request. - The controller may need some info from the db to fulfill the request. Access to the database is handled through the 'model', a python representation of the tables in the database (one class per table). - The controller then renders the page (the 'view') by using a template. Template handling is done through the mako package. These templates can refer to python variables, and additionally can contain typical web stuff like CSS and Javascript / jQuery. - In addition to requests for a complete page, parts of a page can be updated in a similar way through AJAX calls. > > Frontend: > - > [..] > > Question: what is the reasoning to both use jQuery and YUI? Can't we > select one framework and do everything in that? > Answer: according to the wiki 'future' list, YUI should be killed in > favor of jQuery. > > Related question: a bird told me that it may be better to use a > higher-level framework like AngularJS instead of directly scripting > stuff either in plain Javascript or jQuery. Has there been any thought > about this before? After reading further, I have the impression that AngularJS is not really necessary as we already have the mako template library, and most of the rendering of a page should be done at that (server-side) level rather than doing it in (client-side) Javascript. > > On the 'future' list, I see "move code from templates to controllers > and from controllers to libs or models". Can you clarify this a bit > more? I assume the first aspect (templates->controllers) refers to the fact that the templates are currently containing a lot of stuff that is not just about displaying the data (the main purpose of the template) and that most of this logic should really be in controllers. This has the nice side effect of making things more easy to test. Not sure what type of code should move from controllers to libs or models, though... > > Question: how is the relation between the REST API (controllers/api) > and the AJAX calls? Is the API used internally in some way, or only > provided for external usage? > > The codemirror JS library (http://codemirror.net/) is used to display > code files. > > The mergely JS library (http://mergely.com) is used to display diffs > of code. Mergely is based on codemirror. > > fontello is a library/service to create a font of symbols (instead of > using icons in image files). Did we create our own font or are we > using an existing one? > 'font awesome' is a specific symbol font. How does it relate to fontello? > > pygments: syntax highlighting > > formencode: form validation (also recommended by Pylons) > > excanvas? > > mousetrap? > > native.history.js? > > > Backend: > > > Revision information is obtained from the VCS library (lib/vcs), which > in case of Mercurial interacts directly with the Mercurial python > classes, a
Re: new commits to Kallithea should be sent to the mailing list
Hi, On Sun, Jan 25, 2015 at 5:49 AM, Sean Farley wrote: > > Thomas De Schampheleire writes: > >> Hi, >> >> Currently, new commits on the Kallithea project silently end up in >> Bitbucket. In order for users to become aware of such changes and to >> create an active community, I believe a mail notification of these >> commits should be sent to the mailing list. >> >> I would prefer one mail per commit, but an alternate style is how >> Mercurial does it: one mail per bunch of commits. >> >> What do you think? > > That's a great idea. Andrew, what do you think? Could you set this up on > the server? I now see commit mails from Our Own Kallithea, but nothing from Bitbucket. >From Bitbucket, the only mails currently sent are those for new issues. But there are no mails for comments on an existing issue, nor are there mails for (new / updated / commented) pull requests. Could this be handled too? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: discussion on issue #77 (review icon position for in-line code comments)
On Mon, Jan 26, 2015 at 4:59 PM, Mads Kiilerich wrote: > On 01/26/2015 02:41 AM, Sean Farley wrote: >> >> Mads Kiilerich writes: >>> >>> I also think it is odd that both Bitbucket, Github and Kallithea shows >>> the line numbers for each line in a diff. It is usually worthless >>> information when reviewing through the web. If using the actual source >>> for reference and reviewing locally, the line number of the chunk is >>> more releant than the line number of every line in the diff. I thus >>> think we could make an even better solution if we didn't use horizontal >>> space for both showing line numbers for every line and also leaving >>> space for 'add comment' and 'link to this line' (and in the future >>> perhaps also for annotate information). Perhaps by overlaying the latter >>> on top of the first when hovering over a line. >> >> Ok, this paragraph lost me. I'm guessing you agree to "have a way to >> link to a hunk" > > > yes ... and also for linking to a specific line. But we do not necessarily > have to use screen space to all the time show a link for every line of code > we show on the screen. > >> but posit that "the line number might not make sense >> (especially on the web)"? > > > Yes, as long as I can link to exactly the right line, it doesn't matter much > what the actual number is. > > (I guess the main use case for showing line numbers next to every line is > when doing pair programming / reviewing using the same screen and wanting to > be able to talk about lines without pointing. When using a review tool you > usually have to point and click at the line anyway - just quoting its number > is a bad hyper reference.) > >> Why wouldn't the line numbers in the first column not match with the >> line numbers in the previous version of the file? Same for the second >> column and the diff'ed version of the file? > > > I don't know why they wouldn't (and I don't see how that question is related > to what I said). (But one common reason for that is that you are looking at > a slightly different version of the file.) > > My point is just that it is a bit inefficient to use screen space to show a > lot of line numbers where each one just one bigger than the previous. There > is not a lot of information in that. Nobody needs an overview of line > numbers like they need an overview of code. You know when you are looking > for a line number, and you always do it like "now I want to know the line > number of this line". Given that we have limited screen space and want to > use it efficiently, I think we should consider if it could make sense to > show something else instead of the line numbers. > >> I'm all for displaying more information (like you mention with annotate) >> ESPECIALLY for a way to click back through history for that given hunk / >> section of a file. That's getting off-topic for this discussion, though. > > > It is still about how we can make the diff view "right" so all the relevant > information and functionality is available and easy to use. We should avoid > making micro optimizations if it doesn't help the bigger picture or > conflicts with it. > Just saw that on github the review icon is shown on top of the vertical line that separates the line number from the code. The icon/clickable area is still fairly large. As far as I can see, this covers all requirements a to e. Random example: https://github.com/AnneTheAgile/build-failure-analyzer-plugin/commit/4c87b6e8cfd656a4c0cefcdb1abb8b38b8cbd753 I haven't checked the implementation yet. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
YUI remainders
Hi Takumi, I noticed that you have sent a pull request in the past: https://bitbucket.org/conservancy/kallithea/pull-request/63/remove-global-shortcut-yuc-and-yuq-v2 This replaces several uses of the YUI (Yahoo user interface library) by jQuery. However, there still remain several YUx calls in Kallithea. My question is: are you currently working on this, or are you planning on working on this? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: YUI remainders
On Sun, Feb 8, 2015 at 10:45 PM, Nick Coghlan wrote: > > On 8 Feb 2015 23:56, "Tymoteusz Jankowski" > wrote: >> >> For datatables there is jqgrid: >> >> http://www.trirand.com/blog/?page_id=5 > > For Beaker-project.org, we switched from our original old server rendered > tables to Backgrid (Backbone in general has proven very useful for > introducing richer client side widgets into an existing application). > > More details: http://backgridjs.com/ Next to jqgrid and backgridjs, there is also datatables: http://www.datatables.net/ It only requires jQuery, no other dependencies, whereas backgridjs (although I have not used it) additionally requires backbone.js and underscore.js. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: feature request: partial-context diff
Hi Jamie,
On Thu, Feb 12, 2015 at 5:34 PM, Jamie Peabody wrote:
> Hi Thomas,
>
> Kallithea looks cool, and I saw the Mergely integration that was done there.
> Looks good. It should be possible to do it with folding. It might not be
> too difficult. I am rather busy right now - but if someone wants to have a
> go at it, I will give my assistance and support.
>
I had a brief look at the codemirror code and manual, and I see two
potentially useful things:
- doc.markText(from: {line, ch}, to: {line, ch}, ?options: object) → TextMarker
which has a 'collapsed' property.
From how I understand it, one should be able to create a marker
between two lines and collapse it.
http://codemirror.net/doc/manual.html#api_marker
- the codefold add-on (http://codemirror.net/doc/manual.html#addon_foldcode)
although this may be less suitable in this case.
Is this more or less what you had in mind?
How to find out which lines are suitable for collapsing, from mergely?
And how/where should one start hooking this into mergely?
Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
templates and controllers
Hi, I have a question about how one should handle the split between controllers and templates. Let's take one particular example: the difference between the list of pull requests of a particular repository (/pull-request) and the list of 'my pull requests'. For 'all pull requests' of a repo, the actual data is rendered from the controller and placed in a variable c.pullrequest_data, which is expanded from the template. The data is loaded statically (no AJAX) and paged at the controller side. For 'my pull requests', the actual data is fetched dynamically, via AJAX, and no paging is performed. Since I want to line up these two lists of very similar data (at a minimum the way it is displayed), I would like to know what you consider the best approach wrt this template/controller split. Other feedback on this topic is also welcome. (Side questions: - what is the 'c' for in c.pullrequest_data, 'controller'? - what is the difference between 'c' and 'h' ? ) Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: templates and controllers
On February 13, 2015 4:21:30 PM CET, Mads Kiilerich wrote: >On 02/13/2015 04:11 PM, Thomas De Schampheleire wrote: >> Hi, > >Hi - I owe you a system overview. it is on my todo list! ;-) Looking forward to it :) > >> I have a question about how one should handle the split between >> controllers and templates. Let's take one particular example: the >> difference between the list of pull requests of a particular >> repository (/pull-request) and the list of 'my pull requests'. >> >> For 'all pull requests' of a repo, the actual data is rendered from >> the controller and placed in a variable c.pullrequest_data, which is >> expanded from the template. The data is loaded statically (no AJAX) >> and paged at the controller side. >> >> For 'my pull requests', the actual data is fetched dynamically, via >> AJAX, and no paging is performed. >> >> Since I want to line up these two lists of very similar data (at a >> minimum the way it is displayed), I would like to know what you >> consider the best approach wrt this template/controller split. > >It relates to what has been discussed recently: Whether we should use >server side templating (which is indexable by bots and easy on the >browser (but might require more bandwidth and have higher latency)) or >if we want a more rich javascript browser app that fetches data from >'web services' and do the 'templating' client side. > >You can argue both ways. I think it is a bit overkill to do 'my pull >requests' dynamically. But in the places where lists can grow long, we >should have 'infinite scroll' where data is loaded as you scroll down >in >the list. Thanks. Then I will line up both implementations and handle it statically for now. When the final word on the framework is said we can rework it. For this static implementation, where one template file 'includes' the other, there are also two variants used: - render the included template from the controller, store it in a variable that is passed in the controller context, and display the variable from the main template. - or let the main template include the other template directly from mako code. What are the benefits of either approach? What is preferred? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Commits from Unity to mainline
Hi Mads, I had a look at the commits from Unity at https://bitbucket.org/Unity-Technologies/kallithea/commits/all and find following commits interesting: https://bitbucket.org/Unity-Technologies/kallithea/commits/1d897e8dfb7990553fc0bb1767f3fd5e00d74e8d unity: pullrequests: don't add repo owner as reviewer in all PRs This may not be desired by all, but could be transformed as follows: provide for each repository a configurable list of default reviewers. By default, this list would be empty. A user wishing the repo owner as default reviewer can do so freely, along with any other 'core reviewers'. https://bitbucket.org/Unity-Technologies/kallithea/commits/4c228d11097b25133932669f11c6c58701bfd6af changelog: change default view to 100, max to 2000 The current list of 20 is indeed pretty short. Similar for the list of pull requests of a repository (currently paged at 10). This could be merged as-is in mainline, no? https://bitbucket.org/Unity-Technologies/kallithea/commits/39576ff556bf355fa023c4b450318f72e1645ba3 comments: linkify hashes and issue tracker references Is there any reason why we could not mainline this one? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: pull requests: default target branch
Hi Mads, On Thu, Jan 29, 2015 at 2:28 PM, Mads Kiilerich wrote: > On 01/29/2015 12:08 PM, Thomas De Schampheleire wrote: >> >> Hi, >> >> When creating a pull request in Kallithea, the tip (or specified >> revision) in the originating repository is compared with the >> destination repo's tip. >> >> When having several release branches, this choice could be confusing: >> suppose someone makes changes on the default branch and creates a pull >> request. If the last change in the target repo happens to be on the >> branch of another release, the list of revisions is huge. The creator >> of the pull request may not immediately realize that he should select >> the default branch explicitly. >> >> There already exists the concept of 'landing revision' but it does not >> seem to be used here. A proposal is: >> - for the source, use the current tip (this is typically the change of >> the developer) >> - for the target, use the landing revision (which could be changed to >> the default branch in my case). >> >> An alternative is: >> - for the source, use the current tip (this is typically the change of >> the developer) >> - for the target, use the same branch as the source is on, if it >> exists, otherwise use the landing revision (or the current tip). > > > Yes, I implemented some heuristics that works for us. The most generic ones > have been upstreamed - in addition to that I have custom hacks to help it > make suggestions based on our branch naming scheme. Could you point me to the relevant commits, or aren't they on https://bitbucket.org/Unity-Technologies/kallithea/commits/all yet? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
pull requests: variable names
Hi, In the pull request code (both controllers and templates) I find the variable names non-intuitive and it is therefore harder to make changes. With this mail I would like to get an agreement on the terminology we can change to. - org vs other --> source/src and dest - c.cs_repo: what does 'cs' stand for here? This could become src_repo - c.a_repo: what does 'a' stand for here?? This could become dest_repo. Other variable names with the same prefixes 'cs'/'a'/'org'/'other' would be changed too, of course. Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
unit testing controllers
Hi,
I would like to add some tests for the pullrequest controller method
_get_repo_refs(), which returns the list of
revisions/branches/tags/... to populate the select boxes on the pull
request creation page.
Most of the current tests are actually parsing the HTTP response data,
but this is an indirect way of testing. I would prefer testing the
function directly.
I'm a bit stuck on setting up the data correctly. I added the
following in tests/functional/test_pullrequests.py:
def test_repo_refs(self):
main = fixture.create_repo('main')
Session.add(main)
Session.commit()
#fork = fixture.create_fork(main, 'fork')
controller = PullrequestsController()
print controller._get_repo_refs(main)
fixture.destroy_repo('main')
but when _get_repo_refs is invoked, an error is thrown:
test_repo_refs
(kallithea.tests.functional.test_pullrequests.TestPullrequestsController)
... ERROR
==
ERROR: test_repo_refs
(kallithea.tests.functional.test_pullrequests.TestPullrequestsController)
--
Traceback (most recent call last):
File
"/home/tdescham/repo/contrib/kallithea-typos/kallithea/tests/functional/test_pullrequests.py",
line 25, in test_repo_refs
print controller._get_repo_refs(main)
File
"/home/tdescham/repo/contrib/kallithea-typos/kallithea/controllers/pullrequests.py",
line 108, in _get_repo_refs
tiprev = repo.tags.get('tip')
AttributeError: 'Repository' object has no attribute 'tags'
I'm guessing that the repository I created is not yet fully created,
or some other setup action is missing.
Any help or feedback is appreciated.
Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: templates and controllers
On Sun, Feb 15, 2015 at 6:03 PM, Mads Kiilerich wrote:
> On 02/13/2015 09:34 PM, Thomas De Schampheleire wrote:
>>
>> For this static implementation, where one template file 'includes' the
>> other, there are also two variants used:
>> - render the included template from the controller, store it in a variable
>> that is passed in the controller context, and display the variable from the
>> main template.
>
>
> I don't think that is a good idea. I don't think pattern it is used that
> much? It is not so MVC-ish.
A quick search reveils:
$ grep -rn '= render' controllers/
controllers/followers.py:56:c.followers_data =
render('/followers/followers_data.html')
controllers/forks.py:126:c.forks_data = render('/forks/forks_data.html')
controllers/pullrequests.py:207:c.pullrequest_data =
render('/pullrequests/pullrequest_data.html')
controllers/summary.py:111:readme_data =
renderer.render(readme.content,
controllers/admin/gists.py:234:rendered =
render('admin/gists/edit.html')
controllers/admin/admin.py:144:c.log_data =
render('admin/admin_log.html')
controllers/journal.py:210:c.journal_data =
render('journal/journal_data.html')
controllers/journal.py:353:c.journal_data =
render('journal/journal_data.html')
Unless you see a good reason why the above is like that, this could be
cleaned up...
>
>> - or let the main template include the other template directly from mako
>> code.
>
>
> +1
> I think it is better to handle all templating in the templates.
And let the controller just call 'render' once, for the 'main'
template to use, right?
Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: pull requests: variable names
On Sun, Feb 15, 2015 at 5:50 AM, Nick Coghlan wrote: > On 15 February 2015 at 06:36, Thomas De Schampheleire > wrote: >> Hi, >> >> In the pull request code (both controllers and templates) I find the >> variable names non-intuitive and it is therefore harder to make >> changes. >> >> With this mail I would like to get an agreement on the terminology we >> can change to. >> >> - org vs other --> source/src and dest >> - c.cs_repo: what does 'cs' stand for here? This could become src_repo >> - c.a_repo: what does 'a' stand for here?? This could become dest_repo. >> >> Other variable names with the same prefixes 'cs'/'a'/'org'/'other' >> would be changed too, of course. > > +1 from me for the general idea of having PR code focus on "src -> > dest", without making assumptions about the "src" being outside the > organisation and the "dest" being inside. Thanks Nick. Note that it's unclear to me whether 'org' refers to 'organization' or 'original'. I was thinking the latter, actually. But all the more a reason to clarify things. Besides pull requests, this terminology is also used for the compare controller, and also used in several models. I assume we don't want to change the models unnecessarily, so we should stick to fixing the controllers, is that correct? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: YUI remainders
On Wed, Feb 11, 2015 at 10:42 PM, Thomas De Schampheleire wrote: > On Sun, Feb 8, 2015 at 10:45 PM, Nick Coghlan wrote: >> >> On 8 Feb 2015 23:56, "Tymoteusz Jankowski" >> wrote: >>> >>> For datatables there is jqgrid: >>> >>> http://www.trirand.com/blog/?page_id=5 >> >> For Beaker-project.org, we switched from our original old server rendered >> tables to Backgrid (Backbone in general has proven very useful for >> introducing richer client side widgets into an existing application). >> >> More details: http://backgridjs.com/ > > Next to jqgrid and backgridjs, there is also datatables: > http://www.datatables.net/ > It only requires jQuery, no other dependencies, whereas backgridjs > (although I have not used it) additionally requires backbone.js and > underscore.js. Which direction should we go here? Plenty of alternatives, any preference? My first need would be the pull request overview page, but it makes sense to decide on one data table implemention that can work for all use cases, including the existing YUI ones... Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Tasks for the Next Release
Hi, On Sat, Jan 17, 2015 at 10:54 PM, Sean Farley wrote: > Prompted by Mads, I'm emailing the Kallithea list for discussion about > the next release :-) I was wondering what I would to see in the next > release and thought of the following: > > * finish font awesome (some minor bugs and missing logos) > * gravatar > * update hg compatibility to 3.3 (mainly for discovery improvement) > * documentation improvements!! (especially for installation) > > I'm working on my gravatar pull request this weekend but could use some > help with documentation. > > Any thoughts about this list? What is the current status? Shouldn't we make a release anyhow given the recent security issue, even if it is only 0.1.1 ? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: pull requests: variable names
On Thu, Feb 19, 2015 at 11:07 AM, Nick Coghlan wrote: > On 17 February 2015 at 13:13, Mads Kiilerich wrote: >> On 02/14/2015 09:36 PM, Thomas De Schampheleire wrote: >>> >>> Hi, >>> >>> In the pull request code (both controllers and templates) I find the >>> variable names non-intuitive and it is therefore harder to make >>> changes. >> >> >> I have tried to make names that was better than before ... but it seems like >> there is some way to go ;-) >> >>> With this mail I would like to get an agreement on the terminology we >>> can change to. >>> >>> - org vs other --> source/src and dest >> >> >> Yes, that is a bit messy and could use some cleanup. >> >> I think the main challenge is that a pull request and compare in some ways >> are looking at the same thing from different point of views and reuse code >> and templates. The names source and dest are not so obvious in the context >> of compare. But give it a try and see how it works out. (Consider doing the >> two renaming in two different changesets so it is easier to review.) > > Ah, that does indeed make a difference - source/dest isn't right for pure > diffs. > >>> - c.cs_repo: what does 'cs' stand for here? This could become src_repo >> >> >> 'cs' is for changesets, referring to the side where the changes happened. >> >> I think the advantage of the current naming is that it refers to what it >> _is_ not to what we might happen to use it for or how we interpret it. > > Would "changed_repo" be OK? > >>> - c.a_repo: what does 'a' stand for here?? This could become dest_repo. >> >> The 'a' was meant to refer to the ancestor used for comparing. > > And expanding this out to "ancestor_repo"? > > Now you've explained them, I think changed & ancestor make sense as > terminology, but the "cs" and "a" abbreviations are arguably a bit > *too* abbreviated for clarity. > Agreed. Note that 'ancestor_repo' is not necessarily true: when comparing two repos they could both be peers, not ancestor-child, right? Also, in case of compare, change may neither be accurate. I think it's hard to find one combination of terms that matches both compare _and_ pullrequests. Which code is actually shared between both? If the terminology there is neutral, then in both compare and pullrequests the appropriate derived terminology could be mapped on it, for example left/right for compare, and src/dest for pullrequest. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH] repository 'latest changes': truncate first line to avoid table bleed
For commit messages with the first line being very long, the 'latest changes'
table on the repository overview page can 'bleed', so that the commit number
overlaps with the commit status.
Commit 15cb8156b10d732cf39b37a88c656894621c0f54 changed the initial truncate
on 50 characters to a chop at the first newline characters.
In this commit, re-add a truncation of the first line, at 120 characters.
diff --git a/kallithea/templates/changelog/changelog_summary_data.html
b/kallithea/templates/changelog/changelog_summary_data.html
--- a/kallithea/templates/changelog/changelog_summary_data.html
+++ b/kallithea/templates/changelog/changelog_summary_data.html
@@ -38,7 +38,7 @@
-${h.urlify_commit(h.chop_at(cs.message,'\n'),c.repo_name,
h.url('changeset_home',repo_name=c.repo_name,revision=cs.raw_id))}
+${h.urlify_commit(h.truncate(h.chop_at(cs.message,'\n'), 120),
c.repo_name, h.url('changeset_home',repo_name=c.repo_name,revision=cs.raw_id))}
${h.age(cs.date)}
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Commits from Unity to mainline
On Thu, Feb 19, 2015 at 1:59 PM, Mads Kiilerich wrote: > On 02/19/2015 11:13 AM, Nick Coghlan wrote: >> >> On 18 February 2015 at 13:14, Mads Kiilerich wrote: >>> >>> What do others say; would you like to have the "don't add repo owner as >>> reviewer in all PRs" change in Kallithea? >> >> While we're still in the early stages of a comparison between >> Kallithea & Phabricator, > > > I haven't looked at Phrabricator but it is my impression that we could get a > lot of inspiration there. No matter what you end up with, I hope you will > share the pros and cons. > We are also evaluating phabricator for the review aspect (Differential) and my biggest problem with this tool is that it does not properly support separate commits. When sending a review for multiple commits, I want to see each of them individually in one 'review request'. But this is not possible in Phabricator. See also https://secure.phabricator.com/T7316 and the referenced 2012 thread https://secure.phabricator.com/T1508. Of all the tools we have looked at (Crucible, Phabricator, CodeCollaborator, reviewboard, Kallithea), only Kallithea properly supports per-commit review, with a mercurial backend. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH v2] repository summary: avoid table bleed on long commit messages
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424381993 -3600
# Thu Feb 19 22:39:53 2015 +0100
# Node ID f5897d3eddf183efb8a9ea1ab29f2d2792936840
# Parent 1619d9ebc1b9b40379c192ddb45e5802ecfb8f2b
repository summary: avoid table bleed on long commit messages
For commit messages with the first line being very long, the 'latest
changes' table on the repository overview page can 'bleed', so that the
commit number overlaps with the commit status.
Commit 15cb8156b10d732cf39b37a88c656894621c0f54 changed the initial truncate
on 50 characters to a chop at the first newline characters, causing this
issue to pop up more frequently.
Instead of using floating divs for the commit status and number of comments,
use dedicated table columns, as compact as possible.
Additionally, move these new columns to the very left of the table, instead
of cramming them in between the revision and commit message.
The comments-container class gets a new attribute 'white-space: nowrap' to
avoid the comment icon to wrap from the number of comments, when the table
does wrap on a small screen.
Note that the icon currently does not display as it should be renamed from
icon-comment-alt/colored to icon-comment. This will be fixed by Sean Farley.
diff --git a/kallithea/public/css/style.css b/kallithea/public/css/style.css
--- a/kallithea/public/css/style.css
+++ b/kallithea/public/css/style.css
@@ -1417,6 +1417,10 @@
padding: 5px;
}
+#content div.box table td.compact {
+padding: 0;
+}
+
#content div.box table tr.selected td {
background: #FFC;
}
@@ -2581,6 +2585,7 @@
overflow: hidden;
padding: 0;
margin: 0;
+white-space: nowrap;
}
#graph_content .comments-container {
diff --git a/kallithea/templates/changelog/changelog_summary_data.html
b/kallithea/templates/changelog/changelog_summary_data.html
--- a/kallithea/templates/changelog/changelog_summary_data.html
+++ b/kallithea/templates/changelog/changelog_summary_data.html
@@ -2,6 +2,8 @@
%if c.repo_changesets:
+
+
${_('Revision')}
${_('Commit Message')}
${_('Age')}
@@ -10,8 +12,7 @@
%for cnt,cs in enumerate(c.repo_changesets):
-
-
+
%if c.statuses.get(cs.raw_id):
@@ -24,18 +25,21 @@
%endif
%endif
+
+
+
%if c.comments.get(cs.raw_id,[]):
- ${len(c.comments[cs.raw_id])}
+ ${len(c.comments[cs.raw_id])}
%endif
-
+
+
${h.show_id(cs)}
-
${h.urlify_commit(h.chop_at(cs.message,'\n'),c.repo_name,
h.url('changeset_home',repo_name=c.repo_name,revision=cs.raw_id))}
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] repository 'latest changes': truncate first line to avoid table bleed
On Thu, Feb 19, 2015 at 5:01 PM, Mads Kiilerich wrote: > On 02/19/2015 01:36 PM, Thomas De Schampheleire wrote: >> >> For commit messages with the first line being very long, the 'latest >> changes' >> table on the repository overview page can 'bleed', so that the commit >> number >> overlaps with the commit status. >> >> Commit 15cb8156b10d732cf39b37a88c656894621c0f54 changed the initial >> truncate >> on 50 characters to a chop at the first newline characters. >> In this commit, re-add a truncation of the first line, at 120 characters. > > > Wouldn't it be better to do that truncation in css, setting some max width > of the column and hide overflow? > > The 120 seems a bit arbitrary and closely related to the actual styling of > the page (and screen size). > > > 2nd thought after having a closer look, and reproduced it by making the > browser window very narrow: > > The root cause of this seems to be that our templates / styling has an > annoying addiction to 'float'. > > The right fix would be to do something to this changeset-status-container. > Perhaps give it is own table column ... or at least set a min width on the > changeset column ... or give the pre with the hash a margin-right:25px . > You're right, the 120 truncate is a bit of a hack. I just sent a v2 that uses dedicated columns, which works fine even on small windows. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH] changelog: fix display artifacts in expanded multi-line commit messages
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424425442 -3600
# Fri Feb 20 10:44:02 2015 +0100
# Node ID 34ebcf88c211c1bc384c67b2d2410dae7096281f
# Parent a9658ed8313565210debf93e7af7b282679b1301
changelog: fix display artifacts in expanded multi-line commit messages
When a multi-line commit message is expanded in a changelog, and the last line
of the commit message has some characters that go below the baseline, like an
underscore, g, j, y, ... the bottom part of these characters would not be
shown.
This is caused by the 'overflow: hidden' property set on the unexpanded
message. Reset that property by adding 'overflow: initial' on the expanded
class.
Additionally, slightly enlarge the margin of the expanded message box.
diff --git a/kallithea/public/css/style.css b/kallithea/public/css/style.css
--- a/kallithea/public/css/style.css
+++ b/kallithea/public/css/style.css
@@ -2562,7 +2562,8 @@ BIN_FILENODE = 6
#graph_content .container .mid .message.expanded,
#graph_content_pr .compare_view_commits .message.expanded {
height: auto;
-margin: 4px 0px 4px 0px;
+margin: 8px 0px 8px 0px;
+overflow: initial;
}
#graph_content .container .extra-container {
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #99: pull request updates: improve 'description' (conservancy/kallithea)
New issue 99: pull request updates: improve 'description' https://bitbucket.org/conservancy/kallithea/issue/99/pull-request-updates-improve-description Thomas De Schampheleire: Currently, updates to a pull request are marked in one big text area 'description' without structure. The resulting description, which is a combination of the manually entered description and 0 or more auto-generated update blocks, is not very readable. Moreover, when v3 is created, there should only be a reference to v2. The reference to v1 is no longer necessary. Since currently this is free-text being appended, the reference remains. It would be better if the status updates are handled in a separate field, so that the description is only used by the author himself. This will also allow for a more structured update and avoid references to older revisions but the previous one. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #100: gist overview should list file name (conservancy/kallithea)
New issue 100: gist overview should list file name https://bitbucket.org/conservancy/kallithea/issue/100/gist-overview-should-list-file-name Thomas De Schampheleire: Currently, each gist can only contain one file. While that could be improved (multiple files per gist), the overview currently is pretty useless if no gist description was provided, as the only info you see is the gist number. Adding the file name could be useful here. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #101: UI: consistent font sizes (conservancy/kallithea)
New issue 101: UI: consistent font sizes https://bitbucket.org/conservancy/kallithea/issue/101/ui-consistent-font-sizes Thomas De Schampheleire: Kallithea is using a large mix of font sizes, big and small, inconsistently. At least, I don't understand the logic. Examples of illogical font sizes are the repo URLs on the summary page, and the pull requests overview page of a repository. A good UI has a very limited amount of font sizes in use. Most text will be in the normal size, there will be some exceptions in small font, and things like titles would be in a large size. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 0 of 2 RFC] pullrequests: add unittests
Hi, This RFC is the beginning of adding a set of unittests for the existing behavior in _get_repo_refs. I would like to get feedback on the following: a. general feedback on the current code b. the next batch of tests is supposed to verify the behavior of _get_repo_refs for pull requests from a branch. For this I need to add a branch to the repository in the test, but I'm unsure how I should do this. Any help is appreciated here. Similar question for adding bookmarks (hg only?) and tags. Partially related to b. a good explanation on the repository handling in Kallithea is very welcome: there is obviously the real repository on filesystem, but in Kallithea I see different classes: - Repository - RepoModel - BaseRepository How does all of this fit together? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 1 of 2 RFC] tests: move helper commit_change from compare test to fixture
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424291941 -3600
# Wed Feb 18 21:39:01 2015 +0100
# Node ID 50af637aabb506a6afeb44940e1ac498cce8311a
# Parent c9bfc0bc10dc8a36403a2c4d8b8ec81c930b6547
tests: move helper commit_change from compare test to fixture
Move the generic helper function _commit_change() to the fixture class,
allowing reuse by other test classes.
diff --git a/kallithea/tests/fixture.py b/kallithea/tests/fixture.py
--- a/kallithea/tests/fixture.py
+++ b/kallithea/tests/fixture.py
@@ -25,6 +25,8 @@
from kallithea.model.repo_group import RepoGroupModel
from kallithea.model.user_group import UserGroupModel
from kallithea.model.gist import GistModel
+from kallithea.model.scm import ScmModel
+from kallithea.lib.vcs.backends.base import EmptyChangeset
dn = os.path.dirname
FIXTURES = os.path.join(dn(dn(os.path.abspath(__file__))), 'tests', 'fixtures')
@@ -260,3 +262,34 @@
source = source.strip()
return source
+
+def commit_change(self, repo, filename, content, message, vcs_type,
parent=None, newfile=False):
+repo = Repository.get_by_repo_name(repo)
+_cs = parent
+if not parent:
+_cs = EmptyChangeset(alias=vcs_type)
+
+if newfile:
+nodes = {
+filename: {
+'content': content
+}
+}
+cs = ScmModel().create_nodes(
+user=TEST_USER_ADMIN_LOGIN, repo=repo,
+message=message,
+nodes=nodes,
+parent_cs=_cs,
+author=TEST_USER_ADMIN_LOGIN,
+)
+else:
+cs = ScmModel().commit_change(
+repo=repo.scm_instance, repo_name=repo.repo_name,
+cs=parent, user=TEST_USER_ADMIN_LOGIN,
+author=TEST_USER_ADMIN_LOGIN,
+message=message,
+content=content,
+f_path=filename
+)
+return cs
+
diff --git a/kallithea/tests/functional/test_compare.py
b/kallithea/tests/functional/test_compare.py
--- a/kallithea/tests/functional/test_compare.py
+++ b/kallithea/tests/functional/test_compare.py
@@ -3,44 +3,10 @@
from kallithea.model.repo import RepoModel
from kallithea.model.meta import Session
from kallithea.model.db import Repository
-from kallithea.model.scm import ScmModel
-from kallithea.lib.vcs.backends.base import EmptyChangeset
from kallithea.tests.fixture import Fixture
fixture = Fixture()
-
-def _commit_change(repo, filename, content, message, vcs_type, parent=None,
newfile=False):
-repo = Repository.get_by_repo_name(repo)
-_cs = parent
-if not parent:
-_cs = EmptyChangeset(alias=vcs_type)
-
-if newfile:
-nodes = {
-filename: {
-'content': content
-}
-}
-cs = ScmModel().create_nodes(
-user=TEST_USER_ADMIN_LOGIN, repo=repo,
-message=message,
-nodes=nodes,
-parent_cs=_cs,
-author=TEST_USER_ADMIN_LOGIN,
-)
-else:
-cs = ScmModel().commit_change(
-repo=repo.scm_instance, repo_name=repo.repo_name,
-cs=parent, user=TEST_USER_ADMIN_LOGIN,
-author=TEST_USER_ADMIN_LOGIN,
-message=message,
-content=content,
-f_path=filename
-)
-return cs
-
-
def _commit_div(sha, msg):
return """%s""" % (sha, msg)
@@ -66,19 +32,22 @@
cur_user=TEST_USER_ADMIN_LOGIN)
self.r1_id = repo1.repo_id
#commit something !
-cs0 = _commit_change(repo1.repo_name, filename='file1',
content='line1\n',
- message='commit1', vcs_type='hg', parent=None,
newfile=True)
+cs0 = fixture.commit_change(repo1.repo_name, filename='file1',
+content='line1\n', message='commit1', vcs_type='hg',
+parent=None, newfile=True)
#fork this repo
repo2 = fixture.create_fork('one', 'one-fork')
self.r2_id = repo2.repo_id
#add two extra commit into fork
-cs1 = _commit_change(repo2.repo_name, filename='file1',
content='line1\nline2\n',
- message='commit2', vcs_type='hg', parent=cs0)
+cs1 = fixture.commit_change(repo2.repo_name, filename='file1',
+content='line1\nline2\n', message='commit2', vcs_type='hg',
+parent=cs0)
-cs2 = _commit_change(repo2.repo_name, filename='file1',
content='line1\nline2\nline3\n',
- message='commit3', vcs
[PATCH 2 of 2 RFC] pullrequests: add unit tests for _get_repo_refs (unfinished)
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424382374 -3600
# Thu Feb 19 22:46:14 2015 +0100
# Node ID 2ec1df7f244c7e74aa35e1e145e1939da8c25b30
# Parent 50af637aabb506a6afeb44940e1ac498cce8311a
pullrequests: add unit tests for _get_repo_refs (unfinished)
diff --git a/kallithea/tests/functional/test_pullrequests.py
b/kallithea/tests/functional/test_pullrequests.py
--- a/kallithea/tests/functional/test_pullrequests.py
+++ b/kallithea/tests/functional/test_pullrequests.py
@@ -1,5 +1,10 @@
from kallithea.tests import *
+from kallithea.tests.fixture import Fixture
+from kallithea.model.meta import Session
+from kallithea.controllers.pullrequests import PullrequestsController
+
+fixture = Fixture()
class TestPullrequestsController(TestController):
@@ -7,3 +12,92 @@
self.log_user()
response = self.app.get(url(controller='pullrequests', action='index',
repo_name=HG_REPO))
+
+class TestPullrequestsGetRepoRefs(TestController):
+
+def setUp(self):
+self.main = fixture.create_repo('main', repo_type='hg')
+Session.add(self.main)
+Session.commit()
+self.c = PullrequestsController()
+
+def tearDown(self):
+fixture.destroy_repo('main')
+Session.commit()
+Session.remove()
+
+def test_repo_refs_empty_repo(self):
+# empty repo with no commits, no branches, no bookmarks, just one tag
+refs, default = self.c._get_repo_refs(self.main.scm_instance)
+self.assertEqual(default,
'tag:null:')
+
+def test_repo_refs_one_commit_no_hints(self):
+cs0 = fixture.commit_change(self.main.repo_name, filename='file1',
+content='line1\n', message='commit1', vcs_type='hg',
+parent=None, newfile=True)
+
+refs, default = self.c._get_repo_refs(self.main.scm_instance)
+self.assertEqual(default, 'branch:default:%s' % cs0.raw_id)
+self.assertIn(([('branch:default:%s' % cs0.raw_id, 'default (current
tip)')],
+'Branches'), refs)
+
+def test_repo_refs_one_commit_rev_hint(self):
+cs0 = fixture.commit_change(self.main.repo_name, filename='file1',
+content='line1\n', message='commit1', vcs_type='hg',
+parent=None, newfile=True)
+
+refs, default = self.c._get_repo_refs(self.main.scm_instance,
rev=cs0.raw_id)
+expected = 'branch:default:%s' % cs0.raw_id
+self.assertEqual(default, expected)
+self.assertIn(([(expected, 'default (current tip)')], 'Branches'),
refs)
+
+def test_repo_refs_two_commits_no_hints(self):
+cs0 = fixture.commit_change(self.main.repo_name, filename='file1',
+content='line1\n', message='commit1', vcs_type='hg',
+parent=None, newfile=True)
+cs1 = fixture.commit_change(self.main.repo_name, filename='file2',
+content='line2\n', message='commit2', vcs_type='hg',
+parent=None, newfile=True)
+
+refs, default = self.c._get_repo_refs(self.main.scm_instance)
+expected = 'branch:default:%s' % cs1.raw_id
+self.assertEqual(default, expected)
+self.assertIn(([(expected, 'default (current tip)')], 'Branches'),
refs)
+
+def test_repo_refs_two_commits_rev_hints(self):
+cs0 = fixture.commit_change(self.main.repo_name, filename='file1',
+content='line1\n', message='commit1', vcs_type='hg',
+parent=None, newfile=True)
+cs1 = fixture.commit_change(self.main.repo_name, filename='file2',
+content='line2\n', message='commit2', vcs_type='hg',
+parent=None, newfile=True)
+
+refs, default = self.c._get_repo_refs(self.main.scm_instance,
rev=cs0.raw_id)
+expected = 'rev:%s:%s' % (cs0.raw_id, cs0.raw_id)
+self.assertEqual(default, expected)
+self.assertIn(([(expected, 'Changeset: %s' % cs0.raw_id[0:12])],
'Special'), refs)
+self.assertIn(([('branch:default:%s' % cs1.raw_id, 'default (current
tip)')], 'Branches'), refs)
+
+refs, default = self.c._get_repo_refs(self.main.scm_instance,
rev=cs1.raw_id)
+expected = 'branch:default:%s' % cs1.raw_id
+self.assertEqual(default, expected)
+self.assertIn(([(expected, 'default (current tip)')], 'Branches'),
refs)
+
+def test_repo_refs_two_commits_branch_hint(self):
+cs0 = fixture.comm
[PATCH] my pull requests: fix page title when no site branding is set
# HG changeset patch
# User Thomas De Schampheleire
# Date 1423170039 -3600
# Thu Feb 05 22:00:39 2015 +0100
# Node ID d5ca82fa417e49d1cec4989e895bb95397fccdc2
# Parent 9df497f29cf2538f29440e66013bc7f864395082
my pull requests: fix page title when no site branding is set
Just like any other page, the 'middot' between the page name and site name
should only be visible if a site name has been specified.
diff --git a/kallithea/templates/pullrequests/pullrequest_show_my.html
b/kallithea/templates/pullrequests/pullrequest_show_my.html
--- a/kallithea/templates/pullrequests/pullrequest_show_my.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_my.html
@@ -1,7 +1,10 @@
<%inherit file="/base/base.html"/>
<%def name="title()">
-${_('My Pull Requests')} · ${c.site_name}
+${_('My Pull Requests')}
+%if c.site_name:
+· ${c.site_name}
+%endif
<%def name="breadcrumbs_links()">
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] my pull requests: fix page title when no site branding is set
On Sat, Feb 21, 2015 at 8:48 PM, Thomas De Schampheleire
wrote:
> # HG changeset patch
> # User Thomas De Schampheleire
> # Date 1423170039 -3600
> # Thu Feb 05 22:00:39 2015 +0100
> # Node ID d5ca82fa417e49d1cec4989e895bb95397fccdc2
> # Parent 9df497f29cf2538f29440e66013bc7f864395082
> my pull requests: fix page title when no site branding is set
>
> Just like any other page, the 'middot' between the page name and site name
> should only be visible if a site name has been specified.
>
> diff --git a/kallithea/templates/pullrequests/pullrequest_show_my.html
> b/kallithea/templates/pullrequests/pullrequest_show_my.html
> --- a/kallithea/templates/pullrequests/pullrequest_show_my.html
> +++ b/kallithea/templates/pullrequests/pullrequest_show_my.html
> @@ -1,7 +1,10 @@
> <%inherit file="/base/base.html"/>
>
> <%def name="title()">
> -${_('My Pull Requests')} · ${c.site_name}
> +${_('My Pull Requests')}
> +%if c.site_name:
> +· ${c.site_name}
> +%endif
>
>
> <%def name="breadcrumbs_links()">
Hold this one, it's not inline with a recent change of def->block.
I'll send an update shortly, sorry...
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH v2] my pull requests: fix page title when no site branding is set
# HG changeset patch
# User Thomas De Schampheleire
# Date 1423170039 -3600
# Thu Feb 05 22:00:39 2015 +0100
# Node ID 077945f977a22701f48e4512b42bfea6f342be13
# Parent f5897d3eddf183efb8a9ea1ab29f2d2792936840
my pull requests: fix page title when no site branding is set
Just like any other page, the 'middot' between the page name and site name
should only be visible if a site name has been specified.
diff --git a/kallithea/templates/pullrequests/pullrequest_show_my.html
b/kallithea/templates/pullrequests/pullrequest_show_my.html
--- a/kallithea/templates/pullrequests/pullrequest_show_my.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_my.html
@@ -1,7 +1,10 @@
<%inherit file="/base/base.html"/>
<%block name="title">
-${_('My Pull Requests')} · ${c.site_name}
+${_('My Pull Requests')}
+%if c.site_name:
+· ${c.site_name}
+%endif
<%def name="breadcrumbs_links()">
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #102: 'my pull requests' when not logged in: incorrect handling of login (conservancy/kallithea)
New issue 102: 'my pull requests' when not logged in: incorrect handling of login https://bitbucket.org/conservancy/kallithea/issue/102/my-pull-requests-when-not-logged-in Thomas De Schampheleire: Bug scenario: 1. log out 2. browse to 'my pull requests' 3. you'll be presented with a page that is almost entirely dimmed, with a login screen (also dimmed). This page also repeats the top banner a second time. 4. type credentials and login 5. you are now forwarded to a partial page, without top banner nor css, which thus displays incorrectly. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: templates and controllers
On Tue, Feb 17, 2015 at 4:26 AM, Mads Kiilerich wrote:
> On 02/15/2015 09:45 PM, Thomas De Schampheleire wrote:
>>
>>
>> A quick search reveils:
>>
>> $ grep -rn '= render' controllers/
>> controllers/followers.py:56:c.followers_data =
>> render('/followers/followers_data.html')
>> controllers/forks.py:126:c.forks_data =
>> render('/forks/forks_data.html')
>> controllers/pullrequests.py:207:c.pullrequest_data =
>> render('/pullrequests/pullrequest_data.html')
>> controllers/summary.py:111:readme_data =
>> renderer.render(readme.content,
>> controllers/admin/gists.py:234:rendered =
>> render('admin/gists/edit.html')
>> controllers/admin/admin.py:144:c.log_data =
>> render('admin/admin_log.html')
>> controllers/journal.py:210:c.journal_data =
>> render('journal/journal_data.html')
>> controllers/journal.py:353:c.journal_data =
>> render('journal/journal_data.html')
>>
>> Unless you see a good reason why the above is like that, this could be
>> cleaned up...
>
>
> Agreed, it seems like some cleanup could make templates simpler or "better".
It seems that this pattern is used to be able to return data-only in
partial requests (those loaded via ajax/asynchtml javascript functions
with HTTP_X_PARTIAL_XHR set).
In the case of pullrequests, I don't think this is actually being
using, but for example it is used for the journal.
Not sure what to do with this...
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 1 of 3] pullrequest overview: render data from templates iso controller
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424552694 -3600
# Sat Feb 21 22:04:54 2015 +0100
# Node ID ddadf072c99554bfb9d174664148f77d453912fc
# Parent 077945f977a22701f48e4512b42bfea6f342be13
pullrequest overview: render data from templates iso controller
Currently, the pullrequest controller renders the 'data' part of the
pullrequest overview in a context variable, expanded in the
pullrequest_show_all template. This pattern is used in a few other places,
but mainly useful when that data is also loaded dynamically over ajax, which
is not the case for the pullrequest overview.
Remove this context variable and include the data template directly from
the show_all template directly.
diff --git a/kallithea/controllers/pullrequests.py
b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -204,11 +204,6 @@
c.pullrequests_pager = Page(c.pull_requests, page=p, items_per_page=10)
-c.pullrequest_data = render('/pullrequests/pullrequest_data.html')
-
-if request.environ.get('HTTP_X_PARTIAL_XHR'):
-return c.pullrequest_data
-
return render('/pullrequests/pullrequest_show_all.html')
@LoginRequired()
diff --git a/kallithea/templates/pullrequests/pullrequest_show_all.html
b/kallithea/templates/pullrequests/pullrequest_show_all.html
--- a/kallithea/templates/pullrequests/pullrequest_show_all.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_all.html
@@ -54,7 +54,7 @@
-${c.pullrequest_data}
+<%include file='pullrequest_data.html'/>
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 3 of 3] my pullrequests: line up internal handling with repository pullrequests
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424554186 -3600
# Sat Feb 21 22:29:46 2015 +0100
# Node ID 8ed5da3e4184defac92036cd8f19aa7cb61ba0fb
# Parent 0728fc3662d27756523ae0d03b91caffb1587ad9
my pullrequests: line up internal handling with repository pullrequests
Currently, the data for 'my pullrequests' are loaded dynamically through
ajax, unlike the way 'repository pullrequests' are loaded (statically).
As there is no good reason to have both treated differently, and as dynamic
loading of 'my pullrequests' is not really needed, rework the handling of
the 'my pullrequests' page with the 'repository pullrequests' page.
diff --git a/kallithea/config/routing.py b/kallithea/config/routing.py
--- a/kallithea/config/routing.py
+++ b/kallithea/config/routing.py
@@ -729,10 +729,6 @@
'/my_pullrequests',
controller='pullrequests',
action='show_my', conditions=dict(method=["GET"]))
-rmap.connect('my_pullrequests_data',
- '/my_pullrequests_data',
- controller='pullrequests',
- action='show_my_data', conditions=dict(method=["GET"]))
rmap.connect('pullrequest_comment',
'/{repo_name:.*?}/pull-request-comment/{pull_request_id}',
diff --git a/kallithea/controllers/pullrequests.py
b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -207,17 +207,12 @@
return render('/pullrequests/pullrequest_show_all.html')
@LoginRequired()
-def show_my(self): # my_account_my_pullrequests
-c.show_closed = request.GET.get('pr_show_closed')
-return render('/pullrequests/pullrequest_show_my.html')
-
-@NotAnonymous()
-def show_my_data(self):
-c.show_closed = request.GET.get('pr_show_closed')
+def show_my(self):
+c.closed = request.GET.get('closed') or ''
def _filter(pr):
s = sorted(pr, key=lambda o: o.created_on, reverse=True)
-if not c.show_closed:
+if not c.closed:
s = filter(lambda p: p.status != PullRequest.STATUS_CLOSED, s)
return s
@@ -232,7 +227,7 @@
self.authuser.user_id)\
)
-return render('/pullrequests/pullrequest_show_my_data.html')
+return render('/pullrequests/pullrequest_show_my.html')
@LoginRequired()
@NotAnonymous()
diff --git a/kallithea/templates/pullrequests/pullrequest_show_my.html
b/kallithea/templates/pullrequests/pullrequest_show_my.html
--- a/kallithea/templates/pullrequests/pullrequest_show_my.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_my.html
@@ -23,31 +23,19 @@
${self.breadcrumbs()}
-
-## loaded via AJAX
-${_('Loading...')}
+
+
+%if c.closed:
+${h.link_to(_('Hide closed pull requests (only show open pull
requests)'), h.url('my_pullrequests'))}
+%else:
+${h.link_to(_('Show closed pull requests (in addition to open pull
requests)'), h.url('my_pullrequests',closed=1))}
+%endif
+
-
-pyroutes.register('my_pullrequests_data', "${url('my_pullrequests_data')}",
[]);
-
-var show_pullrequests = function(e){
-
-var url = pyroutes.url('my_pullrequests_data');
-if ($('#show_closed').prop('checked')) {
-var url = pyroutes.url('my_pullrequests_data', {'pr_show_closed':
'1'});
-}
-asynchtml(url, $('#pullrequests_container'), function(){
-// new #show_closed has just been loaded
-$('#show_closed').change(function (e) {
-show_pullrequests(e);
-});
-});
-}
-show_pullrequests()
-
-
+
+<%include file='pullrequest_show_my_data.html'/>
+
-
diff --git a/kallithea/templates/pullrequests/pullrequest_show_my_data.html
b/kallithea/templates/pullrequests/pullrequest_show_my_data.html
--- a/kallithea/templates/pullrequests/pullrequest_show_my_data.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_my_data.html
@@ -1,4 +1,3 @@
-${h.checkbox('show_closed',checked="checked" if c.show_closed else "",
label=_('Show closed pull requests (in addition to open pull requests)'))}
${_('Pull Requests Created by
Me')}
%if c.my_pull_requests:
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 0 of 3] (my) pullrequest overview: line-up and sanitize style
Hi, This patch series is a first step in improving the pull request overviews (both 'my' and 'repo' overviews). The next step is to improve the way data is displayed: - use a fixed order of fields for each PR (curiously enough, on the 'my pullrequests' page, the order of fields is different for the 'created by me' and 'I paricipate in' lists) - improve readability by displaying the data in a table, rather than word-after-word. - add other useful data in the table, like the owner of the pull request. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 2 of 3] pullrequest overview: sanitize display style
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424555122 -3600
# Sat Feb 21 22:45:22 2015 +0100
# Node ID 0728fc3662d27756523ae0d03b91caffb1587ad9
# Parent ddadf072c99554bfb9d174664148f77d453912fc
pullrequest overview: sanitize display style
Remove the unnecessarily large font size on the pullrequest overview to line
up with the styling of 'my pullrequests'.
Additionally, add some whitespace between the 'show closed PRs' link and the
actual list.
diff --git a/kallithea/public/css/style.css b/kallithea/public/css/style.css
--- a/kallithea/public/css/style.css
+++ b/kallithea/public/css/style.css
@@ -4629,9 +4629,6 @@
background: #eee;
}
-div.pr-title {
-font-size: 1.6em;
-}
div.pr-details-title {
font-size: 1.6em;
padding: 5px 0px 5px 10px;
diff --git a/kallithea/templates/pullrequests/pullrequest_data.html
b/kallithea/templates/pullrequests/pullrequest_data.html
--- a/kallithea/templates/pullrequests/pullrequest_data.html
+++ b/kallithea/templates/pullrequests/pullrequest_data.html
@@ -2,7 +2,7 @@
% for pr in c.pullrequests_pager:
-
+
%if pr.last_review_status:
%else:
diff --git a/kallithea/templates/pullrequests/pullrequest_show_all.html
b/kallithea/templates/pullrequests/pullrequest_show_all.html
--- a/kallithea/templates/pullrequests/pullrequest_show_all.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_all.html
@@ -44,7 +44,7 @@
-
+
%if c.closed:
${h.link_to(_('Hide closed pull requests (only show open pull
requests)'), h.url('pullrequest_show_all',repo_name=c.repo_name,from_=c.from_))}
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Unstable nosetests wrt login
Hi, When running nosetests, I sometimes see a few tests failing, inconsistently. The errors are: == ERROR: test_index_with_anonymous_access_disabled (kallithea.tests.functional.test_home.TestHomeController) -- Traceback (most recent call last): File "/home/tdescham/repo/contrib/kallithea/kallithea/tests/functional/test_home.py", line 42, in test_index_with_anonymous_access_disabled status=302) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 759, in get expect_errors=expect_errors) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1121, in do_request self._check_status(status, res) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1160, in _check_status "Bad response: %s (not %s)", res_status, status) AppError: Bad response: 200 OK (not 302) == ERROR: test_repo_summary_with_anonymous_access_disabled (kallithea.tests.functional.test_home.TestHomeController) -- Traceback (most recent call last): File "/home/tdescham/repo/contrib/kallithea/kallithea/tests/functional/test_home.py", line 36, in test_repo_summary_with_anonymous_access_disabled status=302) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 759, in get expect_errors=expect_errors) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1121, in do_request self._check_status(status, res) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1160, in _check_status "Bad response: %s (not %s)", res_status, status) AppError: Bad response: 200 OK (not 302) == ERROR: test_access_not_whitelisted_page_via_api_key_0_none (kallithea.tests.functional.test_login.TestLoginController) -- Traceback (most recent call last): File "/home/tdescham/repo/contrib/kallithea/kallithea/tests/nose_parametrized.py", line 94, in parameterized_expand_helper_helper return func(*(self + args)) File "/home/tdescham/repo/contrib/kallithea/kallithea/tests/functional/test_login.py", line 322, in test_access_not_whitelisted_page_via_api_key status=302) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 759, in get expect_errors=expect_errors) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1121, in do_request self._check_status(status, res) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1160, in _check_status "Bad response: %s (not %s)", res_status, status) AppError: Bad response: 200 OK (not 302) == ERROR: test_access_not_whitelisted_page_via_api_key_1_empty_string (kallithea.tests.functional.test_login.TestLoginController) -- Traceback (most recent call last): File "/home/tdescham/repo/contrib/kallithea/kallithea/tests/nose_parametrized.py", line 94, in parameterized_expand_helper_helper return func(*(self + args)) File "/home/tdescham/repo/contrib/kallithea/kallithea/tests/functional/test_login.py", line 322, in test_access_not_whitelisted_page_via_api_key status=302) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 759, in get expect_errors=expect_errors) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1121, in do_request self._check_status(status, res) File "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", line 1160, in _check_status "Bad response: %s (not %s)", res_status, status) AppError: Bad response: 200 OK (not 302) -- Ran 1507 tests in 700.694s FAILED (SKIP=5, errors=4) So it looks like an access that is supposed to be blocked is allowed anyhow. Running the tests again typically 'solves' the proble
Re: [PATCH 3 of 3] my pullrequests: line up internal handling with repository pullrequests
On February 21, 2015 10:48:15 PM CET, Thomas De Schampheleire
wrote:
># HG changeset patch
># User Thomas De Schampheleire
>
># Date 1424554186 -3600
># Sat Feb 21 22:29:46 2015 +0100
># Node ID 8ed5da3e4184defac92036cd8f19aa7cb61ba0fb
># Parent 0728fc3662d27756523ae0d03b91caffb1587ad9
>my pullrequests: line up internal handling with repository pullrequests
>
>Currently, the data for 'my pullrequests' are loaded dynamically
>through
>ajax, unlike the way 'repository pullrequests' are loaded (statically).
>
>As there is no good reason to have both treated differently, and as
>dynamic
>loading of 'my pullrequests' is not really needed, rework the handling
>of
>the 'my pullrequests' page with the 'repository pullrequests' page.
>
>diff --git a/kallithea/config/routing.py b/kallithea/config/routing.py
>--- a/kallithea/config/routing.py
>+++ b/kallithea/config/routing.py
>@@ -729,10 +729,6 @@
> '/my_pullrequests',
> controller='pullrequests',
> action='show_my', conditions=dict(method=["GET"]))
>-rmap.connect('my_pullrequests_data',
>- '/my_pullrequests_data',
>- controller='pullrequests',
>- action='show_my_data',
>conditions=dict(method=["GET"]))
>
> rmap.connect('pullrequest_comment',
> '/{repo_name:.*?}/pull-request-comment/{pull_request_id}',
>diff --git a/kallithea/controllers/pullrequests.py
>b/kallithea/controllers/pullrequests.py
>--- a/kallithea/controllers/pullrequests.py
>+++ b/kallithea/controllers/pullrequests.py
>@@ -207,17 +207,12 @@
> return render('/pullrequests/pullrequest_show_all.html')
>
> @LoginRequired()
>-def show_my(self): # my_account_my_pullrequests
>-c.show_closed = request.GET.get('pr_show_closed')
>-return render('/pullrequests/pullrequest_show_my.html')
>-
>-@NotAnonymous()
This @NotAnonymous should move to the show_my method now. This will presumably
also solve the issue I recently opened regarding the dimmed login screen and
incorrect redirect on my pull requests.
>-def show_my_data(self):
>-c.show_closed = request.GET.get('pr_show_closed')
>+def show_my(self):
>+c.closed = request.GET.get('closed') or ''
>
> def _filter(pr):
> s = sorted(pr, key=lambda o: o.created_on, reverse=True)
>-if not c.show_closed:
>+if not c.closed:
> s = filter(lambda p: p.status != PullRequest.STATUS_CLOSED, s)
> return s
>
>@@ -232,7 +227,7 @@
> self.authuser.user_id)\
> )
>
>-return render('/pullrequests/pullrequest_show_my_data.html')
>+return render('/pullrequests/pullrequest_show_my.html')
>
> @LoginRequired()
> @NotAnonymous()
Best regards,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: templates and controllers
On Sun, Feb 22, 2015 at 4:56 PM, Mads Kiilerich wrote:
> On 02/21/2015 09:35 PM, Thomas De Schampheleire wrote:
>>
>> On Tue, Feb 17, 2015 at 4:26 AM, Mads Kiilerich
>> wrote:
>>>
>>> On 02/15/2015 09:45 PM, Thomas De Schampheleire wrote:
>>>>
>>>>
>>>> A quick search reveils:
>>>>
>>>> $ grep -rn '= render' controllers/
>>>> controllers/followers.py:56:c.followers_data =
>>>> render('/followers/followers_data.html')
>>>> controllers/forks.py:126:c.forks_data =
>>>> render('/forks/forks_data.html')
>>>> controllers/pullrequests.py:207:c.pullrequest_data =
>>>> render('/pullrequests/pullrequest_data.html')
>>>> controllers/summary.py:111:readme_data =
>>>> renderer.render(readme.content,
>>>> controllers/admin/gists.py:234:rendered =
>>>> render('admin/gists/edit.html')
>>>> controllers/admin/admin.py:144:c.log_data =
>>>> render('admin/admin_log.html')
>>>> controllers/journal.py:210:c.journal_data =
>>>> render('journal/journal_data.html')
>>>> controllers/journal.py:353:c.journal_data =
>>>> render('journal/journal_data.html')
>>>>
>>>> Unless you see a good reason why the above is like that, this could be
>>>> cleaned up...
>>>
>>>
>>> Agreed, it seems like some cleanup could make templates simpler or
>>> "better".
>>
>> It seems that this pattern is used to be able to return data-only in
>> partial requests (those loaded via ajax/asynchtml javascript functions
>> with HTTP_X_PARTIAL_XHR set).
>>
>> In the case of pullrequests, I don't think this is actually being
>> using, but for example it is used for the journal.
>>
>> Not sure what to do with this...
>
>
> We could make it simpler and always load the data async, also in the initial
> page load. We will have less code paths to consider if we always do it the
> same way. I would prefer that.
One concern is how usable/functional Kallithea is when Javascript is
disabled in the browser.
Good practice says that the core functionality should still work, so
loading async for the initial code would not be good, unless you also
handle something in .
Then the principle described in
http://pylons-webframework.readthedocs.org/en/latest/helpers.html#partial-updates-with-ajax
could be better: have the initial load include the data template from
pure mako, and let the pager load new pages dynamically. When
javascript is disabled, everything still works.
>
> As a simple short term improvement we could replace c.journal_data in the
> template with an include of journal_data.html .
Yes, similar to what I did in my last pullrequest series right?
There, I could get rid of the variable c.foo_data altogether as the
dynamic aspect was not used.
But in the case of journal, since dynamic loading is used, the
variable still would exist.
I think the way to solve this differently, is to have two controller
methods, one serving the full page and one the data portion. This is
how 'my pullrequests' was handled initially (before my patches).
Then you don't need a magic 'partial' variable in the HTTP request.
Is this correct reasoning?
>
> One future option could be to just return the data as json and generate the
> dom on the client side.
Yes, could be, but again I think we should be careful in relying on
Javascript for core display of data, as mentioned above.
Best regards,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] my pull requests: fix page title when no site branding is set
On Sun, Feb 22, 2015 at 4:44 PM, Mads Kiilerich wrote: > On 02/21/2015 08:48 PM, Thomas De Schampheleire wrote: >> >> # HG changeset patch >> # User Thomas De Schampheleire >> >> # Date 1423170039 -3600 >> # Thu Feb 05 22:00:39 2015 +0100 >> # Node ID d5ca82fa417e49d1cec4989e895bb95397fccdc2 >> # Parent 9df497f29cf2538f29440e66013bc7f864395082 >> my pull requests: fix page title when no site branding is set >> >> Just like any other page, the 'middot' between the page name and site name >> should only be visible if a site name has been specified. > > > It seems like it would be even better to remove the duplication of code and > move all the site_name header handling to root.html? Yes, sounds good, I don't immediately see a problem with that. I will send a patch for that... > > (Next step could be to also show repo_name if there is one in the context.) I would need to look at how do determine this... Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] my pull requests: fix page title when no site branding is set
On Sun, Feb 22, 2015 at 4:44 PM, Mads Kiilerich wrote: > On 02/21/2015 08:48 PM, Thomas De Schampheleire wrote: >> >> # HG changeset patch >> # User Thomas De Schampheleire >> >> # Date 1423170039 -3600 >> # Thu Feb 05 22:00:39 2015 +0100 >> # Node ID d5ca82fa417e49d1cec4989e895bb95397fccdc2 >> # Parent 9df497f29cf2538f29440e66013bc7f864395082 >> my pull requests: fix page title when no site branding is set >> >> Just like any other page, the 'middot' between the page name and site name >> should only be visible if a site name has been specified. > > > It seems like it would be even better to remove the duplication of code and > move all the site_name header handling to root.html? > > (Next step could be to also show repo_name if there is one in the context.) I had a brief look to this second step: templates are currently adding context variables explicitly when appropriate. This could be a repository name, but also a username, user group, repo group, ... How do you suggest to handle this? In the case of repository name for example, the string is typically '%s $title' % c.repo_name, but should we assume this is always the case and that '$title %s' may not be more appropriate? And does it make sense to treat the repository context from root.html, but other relevant context like username from the template itself? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] my pull requests: fix page title when no site branding is set
On Mon, Feb 23, 2015 at 3:57 PM, Mads Kiilerich wrote: > On 02/22/2015 10:19 PM, Thomas De Schampheleire wrote: >> >> On Sun, Feb 22, 2015 at 4:44 PM, Mads Kiilerich >> wrote: >>> >>> On 02/21/2015 08:48 PM, Thomas De Schampheleire wrote: >>>> >>>> # HG changeset patch >>>> # User Thomas De Schampheleire >>>> >>>> # Date 1423170039 -3600 >>>> # Thu Feb 05 22:00:39 2015 +0100 >>>> # Node ID d5ca82fa417e49d1cec4989e895bb95397fccdc2 >>>> # Parent 9df497f29cf2538f29440e66013bc7f864395082 >>>> my pull requests: fix page title when no site branding is set >>>> >>>> Just like any other page, the 'middot' between the page name and site >>>> name >>>> should only be visible if a site name has been specified. >>> >>> >>> It seems like it would be even better to remove the duplication of code >>> and >>> move all the site_name header handling to root.html? >>> >>> (Next step could be to also show repo_name if there is one in the >>> context.) >> >> I had a brief look to this second step: templates are currently adding >> context variables explicitly when appropriate. This could be a >> repository name, but also a username, user group, repo group, ... >> How do you suggest to handle this? >> In the case of repository name for example, the string is typically >> '%s $title' % c.repo_name, but should we assume this is always the >> case and that '$title %s' may not be more appropriate? >> And does it make sense to treat the repository context from root.html, >> but other relevant context like username from the template itself? > > > Yes. "Pages working in the context of a repo (and usually a > changeset/branch)" is so common and important that I think it makes sense to > have extra support and convenience functions for that. > > It would perhaps be nice to have a separate template for such pages. > https://bitbucket.org/conservancy/kallithea/pull-request/52/replaced-the-old-switch-to-dropdown-with > would also fit in nicely there. When you say 'would fit nicely there' you mean to extract the code added in that pullrequest would be moved away from base.html to this new template? And would you create a separate base_repo.html (or something) or rather inherit from base.html ? > > root.html has some special hacks for using c.repo_name when it is defined. > For now I think it would be fine to handle branding and repo name the same > way. > > (A related "feature request": People often have multiple tabs open and they > only see 10 characters of the page title. They want to see the most > important info there so they can tell them apart. But for some users that > will be the hostname/branding, for others it will be the repo name, for > others it will be the branch name, for others it will be the pull request > number or name (when applicable). I don't see a good solution to that. But > let's keep the branding at the end.) > And here your feature request is to keep the branding at the end? Or would the feature be the configuration of the page title based on some %foo modifiers? ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 3 of 3] my pullrequests: line up internal handling with repository pullrequests
On Mon, Feb 23, 2015 at 4:34 PM, Mads Kiilerich wrote:
> On 02/21/2015 10:48 PM, Thomas De Schampheleire wrote:
>>
>> diff --git
>> a/kallithea/templates/pullrequests/pullrequest_show_my_data.html
>> b/kallithea/templates/pullrequests/pullrequest_show_my_data.html
>> --- a/kallithea/templates/pullrequests/pullrequest_show_my_data.html
>> +++ b/kallithea/templates/pullrequests/pullrequest_show_my_data.html
>> @@ -1,4 +1,3 @@
>> -${h.checkbox('show_closed',checked="checked" if c.show_closed else "",
>> label=_('Show closed pull requests (in addition to open pull requests)'))}
>
>
> This option is removed? or is it replaced by something else?
It is replaced by the non-checkbox text link as used on repository
pull requests.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH] templates: move site branding in page title to base template
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424725898 -3600
# Mon Feb 23 22:11:38 2015 +0100
# Node ID 289eb91740c05785c239df947a41c8bbe351c067
# Parent 1619d9ebc1b9b40379c192ddb45e5802ecfb8f2b
templates: move site branding in page title to base template
Instead of repeating the same three lines in each and every template, move
it to the base template.
diff --git a/kallithea/templates/about.html b/kallithea/templates/about.html
--- a/kallithea/templates/about.html
+++ b/kallithea/templates/about.html
@@ -2,9 +2,6 @@
<%inherit file="/base/base.html"/>
<%block name="title">
${_('About')}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs()">
${c.site_name}
diff --git a/kallithea/templates/admin/admin.html
b/kallithea/templates/admin/admin.html
--- a/kallithea/templates/admin/admin.html
+++ b/kallithea/templates/admin/admin.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('Admin Journal')}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs_links()">
diff --git a/kallithea/templates/admin/auth/auth_settings.html
b/kallithea/templates/admin/auth/auth_settings.html
--- a/kallithea/templates/admin/auth/auth_settings.html
+++ b/kallithea/templates/admin/auth/auth_settings.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('Authentication Settings')}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs_links()">
diff --git a/kallithea/templates/admin/defaults/defaults.html
b/kallithea/templates/admin/defaults/defaults.html
--- a/kallithea/templates/admin/defaults/defaults.html
+++ b/kallithea/templates/admin/defaults/defaults.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('Repository Defaults')}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs_links()">
diff --git a/kallithea/templates/admin/gists/edit.html
b/kallithea/templates/admin/gists/edit.html
--- a/kallithea/templates/admin/gists/edit.html
+++ b/kallithea/templates/admin/gists/edit.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('Edit Gist')} · ${c.gist.gist_access_id}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%block name="js_extra">
diff --git a/kallithea/templates/admin/gists/index.html
b/kallithea/templates/admin/gists/index.html
--- a/kallithea/templates/admin/gists/index.html
+++ b/kallithea/templates/admin/gists/index.html
@@ -9,9 +9,6 @@
%else:
${_('Public Gists')}
%endif
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs_links()">
diff --git a/kallithea/templates/admin/gists/new.html
b/kallithea/templates/admin/gists/new.html
--- a/kallithea/templates/admin/gists/new.html
+++ b/kallithea/templates/admin/gists/new.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('New Gist')}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%block name="js_extra">
diff --git a/kallithea/templates/admin/gists/show.html
b/kallithea/templates/admin/gists/show.html
--- a/kallithea/templates/admin/gists/show.html
+++ b/kallithea/templates/admin/gists/show.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('Gist')} · ${c.gist.gist_access_id}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs_links()">
diff --git a/kallithea/templates/admin/my_account/my_account.html
b/kallithea/templates/admin/my_account/my_account.html
--- a/kallithea/templates/admin/my_account/my_account.html
+++ b/kallithea/templates/admin/my_account/my_account.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('My Account')} ${c.authuser.username}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs_links()">
diff --git a/kallithea/templates/admin/notifications/notifications.html
b/kallithea/templates/admin/notifications/notifications.html
--- a/kallithea/templates/admin/notifications/notifications.html
+++ b/kallithea/templates/admin/notifications/notifications.html
@@ -3,9 +3,6 @@
<%block name="title">
${_('My Notifications')} ${c.authuser.username}
-%if c.site_name:
-· ${c.site_name}
-%endif
<%def name="breadcrumbs_links()">
diff --git a/kallithea/templates/admin/notifications/show_notification.html
b/kallithea/templates/admin/notifications/show_notification.html
--- a/kallithea/templates/admin/notifications/show_notification.html
+++ b/kallithea/templates/admin/notifications/show_notification.html
@@ -3,9 +3,6 @@
<%block name="title">
Re: [PATCH] my pull requests: fix page title when no site branding is set
On Mon, Feb 23, 2015 at 4:21 PM, Mads Kiilerich wrote: [..] >>> >>> Yes. "Pages working in the context of a repo (and usually a >>> changeset/branch)" is so common and important that I think it makes sense >>> to >>> have extra support and convenience functions for that. >>> >>> It would perhaps be nice to have a separate template for such pages. >>> >>> https://bitbucket.org/conservancy/kallithea/pull-request/52/replaced-the-old-switch-to-dropdown-with >>> would also fit in nicely there. >> >> When you say 'would fit nicely there' you mean to extract the code >> added in that pullrequest would be moved away from base.html to this >> new template? >> And would you create a separate base_repo.html (or something) or >> rather inherit from base.html ? > > > Yes, something like that. That would however be scope creep. So let's just > take your initial fix and keep this in mind. A basic version of the initial improvement was just sent. > >>> root.html has some special hacks for using c.repo_name when it is >>> defined. >>> For now I think it would be fine to handle branding and repo name the >>> same >>> way. >>> >>> (A related "feature request": People often have multiple tabs open and >>> they >>> only see 10 characters of the page title. They want to see the most >>> important info there so they can tell them apart. But for some users that >>> will be the hostname/branding, for others it will be the repo name, for >>> others it will be the branch name, for others it will be the pull request >>> number or name (when applicable). I don't see a good solution to that. >>> But >>> let's keep the branding at the end.) >>> >> And here your feature request is to keep the branding at the end? >> Or would the feature be the configuration of the page title based on >> some %foo modifiers? > > > I don't know a good solution but I kind of understand the problem. You can > perhaps help keeping it in mind and see if a solution / improvement pops up. I really think we should write such things down somewhere, instead of having it in your and my mind only. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: templates and controllers
On Mon, Feb 23, 2015 at 4:10 PM, Mads Kiilerich wrote: [..] > >>> As a simple short term improvement we could replace c.journal_data in the >>> template with an include of journal_data.html . >> >> Yes, similar to what I did in my last pullrequest series right? >> There, I could get rid of the variable c.foo_data altogether as the >> dynamic aspect was not used. >> >> But in the case of journal, since dynamic loading is used, the >> variable still would exist. >> I think the way to solve this differently, is to have two controller >> methods, one serving the full page and one the data portion. This is >> how 'my pullrequests' was handled initially (before my patches). >> Then you don't need a magic 'partial' variable in the HTTP request. >> >> Is this correct reasoning? > > > I don't know if it is correct, but yes, I totally agree ;-) > On closer inspection, and comparison with the referenced Pylons docs, I created a patch that gets rid of all the variables, but still does not need to introduce separate foo_data methods. This is in line with what you suggested. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 4 of 5 v2] my pullrequests: line up controller/template handling with repo pullrequests
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424554186 -3600
# Sat Feb 21 22:29:46 2015 +0100
# Node ID 2490789dd26d56d501793516ce32e0df30efa236
# Parent 35d2e797a6884547065c88d5b671f29aff2c5773
my pullrequests: line up controller/template handling with repo pullrequests
Currently, the data for 'my pullrequests' is loaded dynamically through
ajax, unlike the way 'repository pullrequests' are loaded (statically).
As there is no good reason to have both treated differently, and as dynamic
loading of 'my pullrequests' is not really needed, rework the handling of
the 'my pullrequests' page with the 'repository pullrequests' page.
This includes lining up the 'show closed pull requests' checkbox/link.
This also fixes issue #102 ('my pull requests' when not logged in:
incorrect handling of login).
diff --git a/kallithea/config/routing.py b/kallithea/config/routing.py
--- a/kallithea/config/routing.py
+++ b/kallithea/config/routing.py
@@ -729,10 +729,6 @@
'/my_pullrequests',
controller='pullrequests',
action='show_my', conditions=dict(method=["GET"]))
-rmap.connect('my_pullrequests_data',
- '/my_pullrequests_data',
- controller='pullrequests',
- action='show_my_data', conditions=dict(method=["GET"]))
rmap.connect('pullrequest_comment',
'/{repo_name:.*?}/pull-request-comment/{pull_request_id}',
diff --git a/kallithea/controllers/pullrequests.py
b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -210,17 +210,13 @@
return render('/pullrequests/pullrequest_show_all.html')
@LoginRequired()
-def show_my(self): # my_account_my_pullrequests
-c.show_closed = request.GET.get('pr_show_closed')
-return render('/pullrequests/pullrequest_show_my.html')
-
@NotAnonymous()
-def show_my_data(self):
-c.show_closed = request.GET.get('pr_show_closed')
+def show_my(self):
+c.closed = request.GET.get('closed') or ''
def _filter(pr):
s = sorted(pr, key=lambda o: o.created_on, reverse=True)
-if not c.show_closed:
+if not c.closed:
s = filter(lambda p: p.status != PullRequest.STATUS_CLOSED, s)
return s
@@ -235,7 +231,7 @@
self.authuser.user_id)\
)
-return render('/pullrequests/pullrequest_show_my_data.html')
+return render('/pullrequests/pullrequest_show_my.html')
@LoginRequired()
@NotAnonymous()
diff --git a/kallithea/templates/pullrequests/pullrequest_show_my.html
b/kallithea/templates/pullrequests/pullrequest_show_my.html
--- a/kallithea/templates/pullrequests/pullrequest_show_my.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_my.html
@@ -20,31 +20,19 @@
${self.breadcrumbs()}
-
-## loaded via AJAX
-${_('Loading...')}
+
+
+%if c.closed:
+${h.link_to(_('Hide closed pull requests (only show open pull
requests)'), h.url('my_pullrequests'))}
+%else:
+${h.link_to(_('Show closed pull requests (in addition to open pull
requests)'), h.url('my_pullrequests',closed=1))}
+%endif
+
-
-pyroutes.register('my_pullrequests_data', "${url('my_pullrequests_data')}",
[]);
-
-var show_pullrequests = function(e){
-
-var url = pyroutes.url('my_pullrequests_data');
-if ($('#show_closed').prop('checked')) {
-var url = pyroutes.url('my_pullrequests_data', {'pr_show_closed':
'1'});
-}
-asynchtml(url, $('#pullrequests_container'), function(){
-// new #show_closed has just been loaded
-$('#show_closed').change(function (e) {
-show_pullrequests(e);
-});
-});
-}
-show_pullrequests()
-
-
+
+<%include file='pullrequest_show_my_data.html'/>
+
-
diff --git a/kallithea/templates/pullrequests/pullrequest_show_my_data.html
b/kallithea/templates/pullrequests/pullrequest_show_my_data.html
--- a/kallithea/templates/pullrequests/pullrequest_show_my_data.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_my_data.html
@@ -1,4 +1,3 @@
-${h.checkbox('show_closed',checked="checked" if c.show_closed else "",
label=_('Show closed pull requests (in addition to open pull requests)'))}
${_('Pull Requests Created by
Me')}
%if c.my_pull_requests:
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 0 of 5 v2] (my) pullrequest overview: line-up and sanitize style (+misc changes)
Hi, This patch series is a first step in improving the pull request overviews (both 'my' and 'repo' overviews). The next step is to improve the way data is displayed: - use a fixed order of fields for each PR (curiously enough, on the 'my pullrequests' page, the order of fields is different for the 'created by me' and 'I paricipate in' lists) - improve readability by displaying the data in a table, rather than word-after-word. - add other useful data in the table, like the owner of the pull request. In v2: - rework my fixes wrt rendering templates in context variables, and expand to all such usage in Kallithea - fix issue #102 in the third patch - add a trivial patch to remove an unused CSS class Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 3 of 5 v2] pullrequest overview: sanitize display style
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424555122 -3600
# Sat Feb 21 22:45:22 2015 +0100
# Node ID 35d2e797a6884547065c88d5b671f29aff2c5773
# Parent fe50f1415d66f5be1cc05cca0b103d3a502fe6af
pullrequest overview: sanitize display style
Remove the unnecessarily large font size on the pullrequest overview to line
up with the styling of 'my pullrequests'.
Additionally, add some whitespace between the 'show closed PRs' link and the
actual list.
diff --git a/kallithea/public/css/style.css b/kallithea/public/css/style.css
--- a/kallithea/public/css/style.css
+++ b/kallithea/public/css/style.css
@@ -4624,9 +4624,6 @@
background: #eee;
}
-div.pr-title {
-font-size: 1.6em;
-}
div.pr-details-title {
font-size: 1.6em;
padding: 5px 0px 5px 10px;
diff --git a/kallithea/templates/pullrequests/pullrequest_data.html
b/kallithea/templates/pullrequests/pullrequest_data.html
--- a/kallithea/templates/pullrequests/pullrequest_data.html
+++ b/kallithea/templates/pullrequests/pullrequest_data.html
@@ -2,7 +2,7 @@
% for pr in c.pullrequests_pager:
-
+
%if pr.last_review_status:
%else:
diff --git a/kallithea/templates/pullrequests/pullrequest_show_all.html
b/kallithea/templates/pullrequests/pullrequest_show_all.html
--- a/kallithea/templates/pullrequests/pullrequest_show_all.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_all.html
@@ -44,7 +44,7 @@
-
+
%if c.closed:
${h.link_to(_('Hide closed pull requests (only show open pull
requests)'), h.url('pullrequest_show_all',repo_name=c.repo_name,from_=c.from_))}
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 1 of 5 v2] giticon.png: use repotag class to make an icon
# HG changeset patch
# User Sean Farley
# Date 1424214154 28800
# Tue Feb 17 15:02:34 2015 -0800
# Node ID 1619d9ebc1b9b40379c192ddb45e5802ecfb8f2b
# Parent 0cb4a35d8248c8dd94027539dbc1e463d491c403
giticon.png: use repotag class to make an icon
A quick search revealed that some old css code was never used, so it was
removed.
diff --git a/kallithea/public/css/contextbar.css
b/kallithea/public/css/contextbar.css
--- a/kallithea/public/css/contextbar.css
+++ b/kallithea/public/css/contextbar.css
@@ -3,7 +3,6 @@
*/
i.icon-ellipsis-horizontal:after { content: ' ...';}
-i.icon-git { background-image: url('../images/icons/giticon.png');}
i.icon-disabled { background-image: url('../images/icons/shading.png');} /*
todo: use instead of minus sign */
i[class^='icon-'] {
diff --git a/kallithea/public/css/style.css b/kallithea/public/css/style.css
--- a/kallithea/public/css/style.css
+++ b/kallithea/public/css/style.css
@@ -501,13 +501,6 @@
margin: 0px 2px 0px 2px;
}
-#header #header-inner #quick li ul li a.git,
-#header #header-inner #quick li ul li a.git:hover {
-background-image: url("../images/icons/giticon.png");
-padding-left: 42px;
-background-position: 20px 9px;
-}
-
.groups_breadcrumbs a {
color: #fff;
}
diff --git a/kallithea/public/images/icons/giticon.png
b/kallithea/public/images/icons/giticon.png
deleted file mode 100644
index
3f909ea413d66fb362b1991c22c869b36909891c..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
GIT binary patch
literal 0
Hc$@hg
%endif
%if h.is_git(c.db_repo):
-
+ git
%endif
## public/private
@@ -363,7 +363,7 @@
tmpl += 'hg ';
}
else if(obj_dict['repo_type'] === 'git'){
-tmpl += ' ';
+tmpl += 'git ';
}
if(obj_dict['private']){
tmpl += ' ';
diff --git a/kallithea/templates/data_table/_dt_elements.html
b/kallithea/templates/data_table/_dt_elements.html
--- a/kallithea/templates/data_table/_dt_elements.html
+++ b/kallithea/templates/data_table/_dt_elements.html
@@ -70,7 +70,7 @@
%if h.is_hg(rtype):
hg
%elif h.is_git(rtype):
-
+git
%endif
##PRIVATE/PUBLIC
diff --git a/kallithea/tests/functional/test_home.py
b/kallithea/tests/functional/test_home.py
--- a/kallithea/tests/functional/test_home.py
+++ b/kallithea/tests/functional/test_home.py
@@ -20,7 +20,7 @@
response.mustcontain('var data = {"totalRecords": %s' %
len(Repository.getAll()))
response.mustcontain(r'href=\"/%s\"' % HG_REPO)
-response.mustcontain(r'git')
response.mustcontain(r'git"""
)
#public/private
response.mustcontain(
@@ -102,7 +102,7 @@
#repo type
response.mustcontain(
-"""git"""
)
#public/private
response.mustcontain(
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 2 of 5 v2] controllers: don't pass rendered templates in context variables
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424552694 -3600
# Sat Feb 21 22:04:54 2015 +0100
# Node ID fe50f1415d66f5be1cc05cca0b103d3a502fe6af
# Parent 1619d9ebc1b9b40379c192ddb45e5802ecfb8f2b
controllers: don't pass rendered templates in context variables
Some controllers used the followifng pattern:
- render a data template into a context variable
- for partial (ajax) requests, return the contents of this variable
- for full-page requests, render the full page, which expands the value of
the context variable
Instead, avoid context variables let the controller simply render the full or
partial page, and let
the full page template include the partial page.
Remove this context variable for templating and use render exclusively.
>From templates, use %include instead of context variables.
This in line with the suggestions in the Pylons documentation:
http://pylons-webframework.readthedocs.org/en/latest/helpers.html#partial-updates-with-ajax
diff --git a/kallithea/controllers/admin/admin.py
b/kallithea/controllers/admin/admin.py
--- a/kallithea/controllers/admin/admin.py
+++ b/kallithea/controllers/admin/admin.py
@@ -141,8 +141,8 @@
return url.current(filter=c.search_term, **kw)
c.users_log = Page(users_log, page=p, items_per_page=10,
url=url_generator)
-c.log_data = render('admin/admin_log.html')
if request.environ.get('HTTP_X_PARTIAL_XHR'):
-return c.log_data
+return render('admin/admin_log.html')
+
return render('admin/admin.html')
diff --git a/kallithea/controllers/followers.py
b/kallithea/controllers/followers.py
--- a/kallithea/controllers/followers.py
+++ b/kallithea/controllers/followers.py
@@ -53,9 +53,7 @@
.order_by(UserFollowing.follows_from)
c.followers_pager = Page(d, page=p, items_per_page=20)
-c.followers_data = render('/followers/followers_data.html')
-
if request.environ.get('HTTP_X_PARTIAL_XHR'):
-return c.followers_data
+return render('/followers/followers_data.html')
return render('/followers/followers.html')
diff --git a/kallithea/controllers/forks.py b/kallithea/controllers/forks.py
--- a/kallithea/controllers/forks.py
+++ b/kallithea/controllers/forks.py
@@ -123,10 +123,8 @@
d.append(r)
c.forks_pager = Page(d, page=p, items_per_page=20)
-c.forks_data = render('/forks/forks_data.html')
-
if request.environ.get('HTTP_X_PARTIAL_XHR'):
-return c.forks_data
+return render('/forks/forks_data.html')
return render('/forks/forks.html')
diff --git a/kallithea/controllers/journal.py b/kallithea/controllers/journal.py
--- a/kallithea/controllers/journal.py
+++ b/kallithea/controllers/journal.py
@@ -207,9 +207,8 @@
c.journal_pager = Page(journal, page=p, items_per_page=20,
url=url_generator)
c.journal_day_aggreagate = self._get_daily_aggregate(c.journal_pager)
-c.journal_data = render('journal/journal_data.html')
if request.environ.get('HTTP_X_PARTIAL_XHR'):
-return c.journal_data
+return render('journal/journal_data.html')
repos_list = Session().query(Repository)\
.filter(Repository.user_id ==
@@ -350,9 +349,9 @@
c.journal_day_aggreagate = self._get_daily_aggregate(c.journal_pager)
-c.journal_data = render('journal/journal_data.html')
if request.environ.get('HTTP_X_PARTIAL_XHR'):
-return c.journal_data
+return render('journal/journal_data.html')
+
return render('journal/public_journal.html')
@LoginRequired(api_access=True)
diff --git a/kallithea/controllers/pullrequests.py
b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -204,10 +204,8 @@
c.pullrequests_pager = Page(c.pull_requests, page=p, items_per_page=10)
-c.pullrequest_data = render('/pullrequests/pullrequest_data.html')
-
if request.environ.get('HTTP_X_PARTIAL_XHR'):
-return c.pullrequest_data
+return render('/pullrequests/pullrequest_data.html')
return render('/pullrequests/pullrequest_show_all.html')
diff --git a/kallithea/templates/admin/admin.html
b/kallithea/templates/admin/admin.html
--- a/kallithea/templates/admin/admin.html
+++ b/kallithea/templates/admin/admin.html
@@ -30,7 +30,7 @@
-${c.log_data}
+<%include file='admin_log.html'/>
diff --git a/kallithea/templates/followers/followers.html
b/kallithea/templates/followers/followers.html
---
[PATCH 5 of 5 v2] templates: changelog_summary: remove non-existing CSS class table_disp
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424812656 -3600
# Tue Feb 24 22:17:36 2015 +0100
# Node ID c7f97c237dacbed8770d4aacf20feb8175ee8d99
# Parent 2490789dd26d56d501793516ce32e0df30efa236
templates: changelog_summary: remove non-existing CSS class table_disp
The changelog_summary template references a non-existing CSS class
table_disp. Remove it.
diff --git a/kallithea/templates/changelog/changelog_summary_data.html
b/kallithea/templates/changelog/changelog_summary_data.html
--- a/kallithea/templates/changelog/changelog_summary_data.html
+++ b/kallithea/templates/changelog/changelog_summary_data.html
@@ -1,6 +1,6 @@
## -*- coding: utf-8 -*-
%if c.repo_changesets:
-
+
${_('Revision')}
${_('Commit Message')}
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 1 of 5 v2] giticon.png: use repotag class to make an icon
On Tue, Feb 24, 2015 at 10:36 PM, Thomas De Schampheleire wrote: > # HG changeset patch > # User Sean Farley > # Date 1424214154 28800 > # Tue Feb 17 15:02:34 2015 -0800 > # Node ID 1619d9ebc1b9b40379c192ddb45e5802ecfb8f2b > # Parent 0cb4a35d8248c8dd94027539dbc1e463d491c403 > giticon.png: use repotag class to make an icon > [..] Sorry, this patch is already upstream and was inadvertently sent... Please ignore. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
About patches on the mailing list vs bitbucket pull requests
Hi, As you noticed I have been sending patches to the mailing list lately, rather than opening bitbucket pullrequests. I find the feedback/discussion more easy on such e-mailed patches. However, the biggest downside is that it is difficult to see which patches are still to be applied/discussed. In other projects, a tool like patchwork (http://jk.ozlabs.org/projects/patchwork/, for example Buildroot: http://patchwork.ozlabs.org/project/buildroot/list/ ) is used to track patches and their state. How will we proceed with this? A goal that was stated before is to send patches through Our Own Kallithea. What is blocking this currently? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: About patches on the mailing list vs bitbucket pull requests
On Tue, Feb 24, 2015 at 11:37 PM, Mads Kiilerich wrote: > On 02/24/2015 10:45 PM, Thomas De Schampheleire wrote: >> >> Hi, >> >> As you noticed I have been sending patches to the mailing list lately, >> rather than opening bitbucket pullrequests. >> >> I find the feedback/discussion more easy on such e-mailed patches. >> >> However, the biggest downside is that it is difficult to see which >> patches are still to be applied/discussed. >> In other projects, a tool like patchwork >> (http://jk.ozlabs.org/projects/patchwork/, for example Buildroot: >> http://patchwork.ozlabs.org/project/buildroot/list/ ) is used to track >> patches and their state. >> >> How will we proceed with this? > > > I prefer to pull everything down anyway and test it and use it in production > and perhaps tweak it before I push it. > > It is tricky for me to keep track of, but mainly my problem. > > A patchwork bot could perhaps be nice ... especially if it automatically can > remove stuff that has been pushed. patchwork recognizes applied patches and marks them as applied, on the condition that the patch is applied untouched. If you tweak a patch before applying, then you'll have to update patchwork manually. > >> A goal that was stated before is to send patches through Our Own >> Kallithea. What is blocking this currently? > > > I guess we could give known and trusted contributors access to the projects > own instance. It is primarily Andrew who is managing it - I will leave that > to him. > > It would not be perfect for our own use ... but that could be motivation for > fixing and improving it ;-) In this context, one major issues that I see with Kallithea is the flood of e-mails caused by review. We should probably discuss this in length in a separate thread, but I think it should be improved before eating the dog (or was it its food? ;-) ) Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 0 of 5 v2] (my) pullrequest overview: line-up and sanitize style (+misc changes)
On Tue, Feb 24, 2015 at 11:07 PM, Mads Kiilerich wrote: > Thomas, I pushed all your pending changes (I hope) ... except this updated > patch set that I will put in test for a couple of days. Yes confirmed, this series are the only remaining ones at the moment. Note that, as mentioned, this first phase of the series only makes some preliminary changes, it does not improve a lot yet on the UI, but it should not be worse than before. The main goal for me is the second set (in progress) that displays the info in a table. > > Thanks a lot for all the high quality contributions! Thanks for these kind words, also to Sean. It's nice to be able to contribute and be appreciated for that! Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] templates: centre the feed icon vertially in the public journal
On Wed, Feb 25, 2015 at 12:43 PM, Andrew Shadura wrote:
> # HG changeset patch
> # User Andrew Shadura
> # Date 1424864441 -3600
> # Wed Feb 25 12:40:41 2015 +0100
> # Node ID 54ca0422017d790f16135b4da347850c68458c55
> # Parent fc311d8c3997063a8c6020f4e8d32ca77be339e5
> templates: centre the feed icon vertially in the public journal
>
> diff --git a/kallithea/templates/journal/public_journal.html
> b/kallithea/templates/journal/public_journal.html
> --- a/kallithea/templates/journal/public_journal.html
> +++ b/kallithea/templates/journal/public_journal.html
> @@ -19,7 +19,7 @@
>
>
> ${_('Public Journal')}
> -
> +
>
>
> class="icon-rss-squared" style="color: #fa9b39">
At the same time I think we should change the orange color to white,
same as is done on the non-public journal. The orange does not look
nice on the green background.
Best regards,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] docs: improve issue tracker integration docs
Hi Andrew,
On Wed, Feb 25, 2015 at 2:06 PM, Andrew Shadura wrote:
> # HG changeset patch
> # User Andrew Shadura
> # Date 1424869587 -3600
> # Wed Feb 25 14:06:27 2015 +0100
> # Node ID 5b85679c832b1de3471ed3c62a555d22cbad0c33
> # Parent 54ca0422017d790f16135b4da347850c68458c55
> docs: improve issue tracker integration docs
>
> diff --git a/docs/setup.rst b/docs/setup.rst
> --- a/docs/setup.rst
> +++ b/docs/setup.rst
> @@ -448,16 +448,29 @@ uncomment following variables in the ini
> issue_prefix = #
>
> `issue_pat` is the regular expression that will fetch issues from commit
> messages.
Maybe this sentence could be changed as well, something like:
`issue_pat` is the regular expression describing which strings in
commit messages will be treated as issue references. A match group in
parentheses should be used to specify the actual issue id.
> -Default regex will match issues in format of # eg. #300.
> +Default regex matches issues in format of # eg. #300.
I would say:
The default expression matches issues in the format '#', e.g. '#300'.
>
> -Matched issues will be replace with the link specified as `issue_server_link`
> -{id} will be replaced with issue id, and {repo} with repository name.
> -Since the # is striped `issue_prefix` is added as a prefix to url.
> -`issue_prefix` can be something different than # if you pass
> -ISSUE- as issue prefix this will generate an url in format::
> +Matched issues are be replaced with the link specified as `issue_server_link`
are be --> are
> +{id} is replaced with issue id, and {repo} with repository name.
with the issue id
with the repository name
> +Since the # is stripped away, `issue_prefix` is added as a prefix to url.
'to url' is not very clear: the prefix is added to the text of the
link, not the url itself. So maybe:
is added as a prefix to the link text.
> +`issue_prefix` doesn't necessarily need to be #, so if you set issue
I would replace the comma with a colon (:) and then remove the 'so'
> +prefix to ISSUE- this will generate an URL in format::
an URL -> a URL since URL is pronounced 'you-are-el'
>
>https://myissueserver.com/example_repo/issue/300";>ISSUE-300
>
> +If needed, more than one pattern can be specified by appending a unique
> suffix to
> +the variables. For example::
> +
> +issue_pat_wiki = (?:wiki-)(.+)
> +issue_server_link_wiki = https://mywiki.com/{id}
> +issue_prefix_wiki = WIKI-
> +
> +From now on, wiki pages can be referenced as wiki-some-id, and every such
> reference
'From now on' -> 'With these settings'
> +will be transformed into::
> +
> + https://mywiki.com/some-id";>WIKI-some-id
> +
> +
Best regards,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] docs: improve issue tracker integration docs
On Wed, Feb 25, 2015 at 2:47 PM, Andrew Shadura wrote:
> # HG changeset patch
> # User Andrew Shadura
> # Date 1424869587 -3600
> # Wed Feb 25 14:06:27 2015 +0100
> # Node ID 27dc7c640192200467740c7fbd9256972ddd9e20
> # Parent 730768e621dcc5bd69c75e34958e90664e8b8162
> docs: improve issue tracker integration docs
>
> diff --git a/docs/setup.rst b/docs/setup.rst
> old mode 100755
> new mode 100644
> --- a/docs/setup.rst
> +++ b/docs/setup.rst
> @@ -447,17 +447,33 @@ uncomment following variables in the ini
> issue_server_link = https://myissueserver.com/{repo}/issue/{id}
> issue_prefix = #
>
> -`issue_pat` is the regular expression that will fetch issues from commit
> messages.
> -Default regex will match issues in format of # eg. #300.
> +`issue_pat` is the regular expression describing which strings in
> +commit messages will be treated as issue references. A match group in
> +parentheses should be used to specify the actual issue id.
>
> -Matched issues will be replace with the link specified as `issue_server_link`
> -{id} will be replaced with issue id, and {repo} with repository name.
> -Since the # is striped `issue_prefix` is added as a prefix to url.
> -`issue_prefix` can be something different than # if you pass
> -ISSUE- as issue prefix this will generate an url in format::
> +The default expression matches issues in the format '#', e.g. '#300'.
> +
> +Matched issues are replaced with the link specified as `issue_server_link`
> +{id} is replaced with issue id, and {repo} with repository name.
> +Since the # is stripped away, `issue_prefix` is prepended to the link text.
> +`issue_prefix` doesn't necessarily need to be #: if you set issue
> +prefix to ISSUE- this will generate a URL in format::
>
>https://myissueserver.com/example_repo/issue/300";>ISSUE-300
>
> +If needed, more than one pattern can be specified by appending a unique
> suffix to
> +the variables. For example::
> +
> +issue_pat_wiki = (?:wiki-)(.+)
> +issue_server_link_wiki = https://mywiki.com/{id}
> +issue_prefix_wiki = WIKI-
> +
> +With these settings, wiki pages can be referenced as wiki-some-id, and every
> +such reference will be transformed into::
> +
> + https://mywiki.com/some-id";>WIKI-some-id
> +
> +
> Hook management
> ---
>
Looks good to me, thanks!
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Unstable nosetests wrt login
On Sun, Feb 22, 2015 at 5:06 PM, Mads Kiilerich wrote: > On 02/22/2015 12:29 PM, Thomas De Schampheleire wrote: >> >> Hi, >> >> When running nosetests, I sometimes see a few tests failing, >> inconsistently. The errors are: >> >> == >> ERROR: test_index_with_anonymous_access_disabled >> (kallithea.tests.functional.test_home.TestHomeController) >> -- >> Traceback (most recent call last): >>File >> "/home/tdescham/repo/contrib/kallithea/kallithea/tests/functional/test_home.py", >> line 42, in test_index_with_anonymous_access_disabled >> status=302) >>File >> "/home/tdescham/repo/contrib/kallithea/dist/v/local/lib/python2.7/site-packages/WebTest-1.4.3-py2.7.egg/webtest/app.py", >> line 759, in get >> expect_errors=expect_errors) >> ... >> -- >> Ran 1507 tests in 700.694s >> >> FAILED (SKIP=5, errors=4) >> >> >> So it looks like an access that is supposed to be blocked is allowed >> anyhow. >> >> Running the tests again typically 'solves' the problem. >> I tried running the offending tests over and over again, but it does >> not reproduce. >> >> I have seen this issue several times already, but very inconsistently. >> >> Is anyone else seeing this? >> Is it a test problem or rather a Kallithea bug? > > > I might have seen it a few times but not so often or reproducible that I can > reproduce it. > > The tests are not unit tests and are not isolated and cannot be run in > parallel ... but we are always running them in the same sequence so it > should be stable. > Mads mentioned on IRC that he has been seeing these issues too now. I have updated to 0.1 to check if the issue was present there too, and it's the case. A script that can be used to detect the problem is: #!/bin/bash i=0; while true; do echo "RUN $i"; let 'i+=1'; nosetests \ kallithea.tests.functional.test_feed \ kallithea.tests.functional.test_files \ kallithea.tests.functional.test_followers \ kallithea.tests.functional.test_forks \ kallithea.tests.functional.test_home; done If you run this, send the output to a logfile, and grep on 'RUN|FAILED' then you will see the errors when they pop up. The special thing about the tests I see is that they use fixture.anon_access, which is implemented as: def anon_access(self, status): """ Context process for disabling anonymous access. use like: fixture = Fixture() with fixture.anon_access(False): #tests after this block anon access will be set to `not status` """ class context(object): def __enter__(self): anon = User.get_default_user() anon.active = status Session().add(anon) Session().commit() time.sleep(1.5) # must sleep for cache (1s to expire) def __exit__(self, exc_type, exc_val, exc_tb): anon = User.get_default_user() anon.active = not status Session().add(anon) Session().commit() return context() The suspicious item in this code is: "time.sleep(1.5) # must sleep for cache (1s to expire)" I am not convinced about the correctness of sleeping for some arbitrary time to let a cache expire. Invalidating the cache explicitly would yield deterministic results. Which cache is this really? Any idea on how to invalidate it? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 0 of 5 v2] (my) pullrequest overview: line-up and sanitize style (+misc changes)
On Tue, Feb 24, 2015 at 11:07 PM, Mads Kiilerich wrote: > Thomas, I pushed all your pending changes (I hope) ... except this updated > patch set that I will put in test for a couple of days. > > Thanks a lot for all the high quality contributions! > I forgot to thank you for the valuable feedback you are giving on each of my patches: it is this feedback that drives me into a certain direction or new code cleanup. So keep it coming! ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 0 of 1] Bad input handling: discussion
On Thu, Feb 26, 2015 at 4:50 PM, Andrew Shadura wrote: > In [1], Mads and I had a discussion on what to do with bad input we may > sometimes receive. My idea is that we should use everything we know how > to handle and ignore what we can't. Mads, if I understood him correctly, > insists we should be conservative in what we accept and just complain if > someone's trying to feed us something we can't handle. > > What's your opinion on this? > > [1]: > https://bitbucket.org/conservancy/kallithea/commits/cc1ab5ef6686526b7aad8c0c190a5c2944e92ecf#Lkallithea/controllers/changeset.pyT78 > I don't think Kallithea should crash or present 500 in cases where an input is not what we expect. In any case, '400 bad request' is better than '500 server error', as also said by Mads in [1]. Whether or not we should ignore invalid input: my initial thought was that it is good idea. However, from the link Mads provided in [1], it seems there can be security issues with such behavior, in general. So I'm not sure anymore what to do here, I'm not very familiar with this area. What could be the reason for such invalid input, other than malicious attempts? ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Issue #104: redirects for logins should remember GET arguments (conservancy/kallithea)
New issue 104: redirects for logins should remember GET arguments https://bitbucket.org/conservancy/kallithea/issue/104/redirects-for-logins-should-remember-get Thomas De Schampheleire: Scenario: - logout - go to changelog of a repository - select a particular revision and click 'create a pull request from this revision' - you will get a login screen - after login, you get to the 'create pull request' page, but the revision selection is gone (the tip is used). A similar problem is possibly present in other places. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH] date representation: use ISO8601 rather than a specific locale
# HG changeset patch
# User Thomas De Schampheleire
# Date 1424987487 -3600
# Thu Feb 26 22:51:27 2015 +0100
# Node ID d80090d1053468f6b84eb8ef41c62446995ddb73
# Parent c7f97c237dacbed8770d4aacf20feb8175ee8d99
date representation: use ISO8601 rather than a specific locale
Dates, in particular in technical systems like Kallithea, are
better shown in a clear concise format like ISO8601 (-MM-DD)
than in a verbose format like 'Thu, Feb 26 2015'.
This commit changes all dates to ISO8601.
---
If desired, we could create two functions: one that returns ISO format and
another for the locale format. Depending on the usage, one or the other is
shown. I'm not very fond of that though, it looks inconsistent.
diff --git a/kallithea/lib/helpers.py b/kallithea/lib/helpers.py
--- a/kallithea/lib/helpers.py
+++ b/kallithea/lib/helpers.py
@@ -469,7 +469,7 @@
def fmt_date(date):
if date:
-_fmt = u"%a, %d %b %Y %H:%M:%S".encode('utf8')
+_fmt = u"%Y-%m-%d %H:%M:%S".encode('utf8')
return date.strftime(_fmt).decode('utf8')
return ""
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH] date representation: use ISO8601 rather than a specific locale
On Thu, Feb 26, 2015 at 10:51 PM, Thomas De Schampheleire
wrote:
> # HG changeset patch
> # User Thomas De Schampheleire
> # Date 1424987487 -3600
> # Thu Feb 26 22:51:27 2015 +0100
> # Node ID d80090d1053468f6b84eb8ef41c62446995ddb73
> # Parent c7f97c237dacbed8770d4aacf20feb8175ee8d99
> date representation: use ISO8601 rather than a specific locale
>
> Dates, in particular in technical systems like Kallithea, are
> better shown in a clear concise format like ISO8601 (-MM-DD)
> than in a verbose format like 'Thu, Feb 26 2015'.
>
> This commit changes all dates to ISO8601.
>
> ---
> If desired, we could create two functions: one that returns ISO format and
> another for the locale format. Depending on the usage, one or the other is
> shown. I'm not very fond of that though, it looks inconsistent.
>
> diff --git a/kallithea/lib/helpers.py b/kallithea/lib/helpers.py
> --- a/kallithea/lib/helpers.py
> +++ b/kallithea/lib/helpers.py
> @@ -469,7 +469,7 @@
>
> def fmt_date(date):
> if date:
> -_fmt = u"%a, %d %b %Y %H:%M:%S".encode('utf8')
> +_fmt = u"%Y-%m-%d %H:%M:%S".encode('utf8')
> return date.strftime(_fmt).decode('utf8')
>
> return ""
In some way related: certain dates in Kallithea are shown as an age '5
days ago' rather than an expanded date.
What is the strategy: when should we use what? Or should we always
show the age with the date as tooltip?
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Kill RST
Hi, The subject should be non-ambiguous but let me add a little more body to it: currently comments and pull request descriptions are parsed as RST (restructured text). From my perspective (and that of several others) this is counterproductive and not necessary: - when typing a comment or PR description, the fact that newlines are not preserved because deemed unimportant by RST is counter-intuitive and above all annoying - having to remember how your plain text would be parsed by RST and having to work around this explicitly is counterproductive. - the average user does not need the features RST provides. So as far as I'm concerned, RST can be removed entirely. The same has been done in Unity's tree with commit https://bitbucket.org/Unity-Technologies/kallithea/commits/09286e5ca064de6930d5bdefb9df6708eda19976 However, I assume that some other people will find this too harsh, so we can think of compromises, for example: - let a global per-project or per-repo setting define whether RST is used for either comments, pull request descriptions, both, or neither. and/or - add a per-comment option that allows/disallows RST, so the user himself can decide whether he wants or does not want formatting. There may be other alternatives, so please share your thoughts. Of course, if there is a consensus to disable RST altogether, then all the better :-) Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Kill RST
On March 2, 2015 7:35:36 PM CET, Sean Farley wrote: > >Thomas De Schampheleire writes: > >> Hi, >> >> The subject should be non-ambiguous but let me add a little more body >> to it: currently comments and pull request descriptions are parsed as >> RST (restructured text). From my perspective (and that of several >> others) this is counterproductive and not necessary: >> >> - when typing a comment or PR description, the fact that newlines are >> not preserved because deemed unimportant by RST is counter-intuitive >> and above all annoying >> >> - having to remember how your plain text would be parsed by RST and >> having to work around this explicitly is counterproductive. > >Yeah, I definitely agree with this. > >> - the average user does not need the features RST provides. >> >> So as far as I'm concerned, RST can be removed entirely. >> The same has been done in Unity's tree with commit >> >https://bitbucket.org/Unity-Technologies/kallithea/commits/09286e5ca064de6930d5bdefb9df6708eda19976 >> >> However, I assume that some other people will find this too harsh, so >> we can think of compromises, for example: > >Why not make RST 'just work'? i.e. if a user types plain text the >output >is plain text. How do you distinguish plain text from RST? ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Kill RST
On Tue, Mar 3, 2015 at 7:57 AM, Andrew Shadura wrote: > Hello, > > On Mon, 02 Mar 2015 10:35:36 -0800 > Sean Farley > wrote: > >> > - the average user does not need the features RST provides. >> > >> > So as far as I'm concerned, RST can be removed entirely. >> > The same has been done in Unity's tree with commit >> > https://bitbucket.org/Unity-Technologies/kallithea/commits/09286e5ca064de6930d5bdefb9df6708eda19976 >> > >> > However, I assume that some other people will find this too harsh, >> > so we can think of compromises, for example: > >> Why not make RST 'just work'? i.e. if a user types plain text the >> output is plain text. > > I think we need to: > > i) switch to some flavour of Markdown by default > ii) let users choose the format of every comment they add: plain text, > md, rst > > We can't detect the markup effectively because both rst and md *are* > plain text (well, md more so). > I'm fine with this approach, but I'd like to either see - plain text being the default in the comment handling, or - allow the user to set this as a personal preference Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
what is the issue with database changes?
Hi, Regularly I hear that we don't want to change the model yet to stay backwards compatible. However, I do see several 'dbmigrate' scripts in the source base, which hint at it being possible to migrate across database changes. Can someone explain in more detail why we do not want such database changes (yet)? Thanks, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 2 of 6] style: add class 'normal-indent' instead of repeated explicit margins
# HG changeset patch
# User Thomas De Schampheleire
# Date 1425415064 -3600
# Tue Mar 03 21:37:44 2015 +0100
# Node ID a076460a649e0e6fd03a530512fbcd838fe52ee5
# Parent c3adb366628c8e7a5cfa016801a7c2f44abc312b
style: add class 'normal-indent' instead of repeated explicit margins
Add a new CSS class for the standard indentation inside the main box,
instead of repeating 'style="..."' statements on the relevant elements.
Ideally, this class should not exist as the necessary padding would be added
to the main box itself, but reworking this is a bigger exercise (to be done
later).
diff --git a/kallithea/public/css/style.css b/kallithea/public/css/style.css
--- a/kallithea/public/css/style.css
+++ b/kallithea/public/css/style.css
@@ -933,6 +933,10 @@
padding-bottom: 2px;
}
+#content div.box div.normal-indent {
+margin: 0 20px 10px 20px;
+}
+
#content div.box p {
color: #5f5f5f;
font-size: 12px;
diff --git a/kallithea/templates/pullrequests/pullrequest_show_all.html
b/kallithea/templates/pullrequests/pullrequest_show_all.html
--- a/kallithea/templates/pullrequests/pullrequest_show_all.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_all.html
@@ -44,7 +44,7 @@
-
+
%if c.closed:
${h.link_to(_('Hide closed pull requests (only show open pull
requests)'), h.url('pullrequest_show_all',repo_name=c.repo_name,from_=c.from_))}
diff --git a/kallithea/templates/pullrequests/pullrequest_show_my.html
b/kallithea/templates/pullrequests/pullrequest_show_my.html
--- a/kallithea/templates/pullrequests/pullrequest_show_my.html
+++ b/kallithea/templates/pullrequests/pullrequest_show_my.html
@@ -20,7 +20,7 @@
${self.breadcrumbs()}
-
+
%if c.closed:
${h.link_to(_('Hide closed pull requests (only show open pull
requests)'), h.url('my_pullrequests'))}
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 0 of 6] pullrequest overview improvements
Hi, This series improves the pullrequests overview as follows: - remove duplication between 'my' and 'repo' pullrequest overview - show info in table rather than unorganized text - add author and originating repo - show age rather than date Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 6 of 6] pullrequest overview: show age rather than date
# HG changeset patch
# User Thomas De Schampheleire
# Date 1425418426 -3600
# Tue Mar 03 22:33:46 2015 +0100
# Node ID 297d798bd5b22ea562d0813bed7e5eb6bc646c1b
# Parent 02c4f6d5f95731f75ddbf196b79100da8d80d46c
pullrequest overview: show age rather than date
Since pullrequests are supposed to be short-lived, the age is more relevant
than the actual date (which is still shown in a tooltip).
diff --git a/kallithea/templates/pullrequests/pullrequest_data.html
b/kallithea/templates/pullrequests/pullrequest_data.html
--- a/kallithea/templates/pullrequests/pullrequest_data.html
+++ b/kallithea/templates/pullrequests/pullrequest_data.html
@@ -14,7 +14,7 @@
${_('Title')}
${_('Author')}
-${_('Date')}
+${_('Age')}
${_('From')}
${_('To')}
@@ -54,7 +54,9 @@
${pr.author.username_and_name}
-${h.fmt_date(pr.created_on)}
+
+ ${h.age(pr.created_on)}
+
<% org_ref_name=pr.org_ref.rsplit(':', 2)[-2] %>
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
