Merge lp:~coopebm1/sloecode/add-wiki-branch into lp:sloecode/wiki
Proposed by
Brendan Cooper
Status: | Needs review |
---|---|
Proposed branch: | lp:~coopebm1/sloecode/add-wiki-branch |
Merge into: | lp:sloecode/wiki |
Diff against target: |
83 lines (+35/-5) 4 files modified
sloecode/bzr/factory.py (+8/-4) sloecode/controllers/admin/project.py (+4/-1) sloecode/controllers/project.py (+13/-0) sloecode/templates/project-wiki.html (+10/-0) |
To merge this branch: | bzr merge lp:~coopebm1/sloecode/add-wiki-branch |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Thomi Richards (community) | Needs Fixing | ||
Review via email:
|
Description of the change
Creates a wiki folder and wiki page.
To post a comment you must log in.
Unmerged revisions
- 152. By Brendan Cooper
-
Seperated Repo and branch creation
- 151. By Sam Bailey
-
Wiki branch is created with new project alongside trunk
- 150. By Sam Bailey
-
project information passed correctly
- 149. By Sam Bailey
-
forgot to add project-wiki.html
- 148. By Sam Bailey
-
Re-directing /p/<name>/wiki to project-wiki.html
Hi,
Sorry for the delay here - I intended to do this last week, but I completely forgot about it.
I'd like you to make some changes to this branch. There are two main things that need fixing:
1) You have duplicated code. In your branch, factory.py lines 72 - 100 look like this:
def create_ shared_ repository_ for_project( self, project_name): project_ repo_dir( project_ name) shared_ repo(dir_ name)
to_ transport = repo.user_ transport. clone(' trunk')
to_ transport = repo._transport .clone( 'trunk' ) format_ registry. make_bzrdir( 'default' ) BzrDir. create_ branch_ convenience(
to_ transport. base, format=format,
possible_ transports= [to_transport] )
branch. set_append_ revisions_ only(True)
"""Create a repository for the project with a trunk branch."""
dir_name = self.get_
repo = create_
# Now to create the trunk.
try:
except AttributeError:
# Fixes lp:948483 bzrlib version 2.1 (which is current in debian
# squeeze) uses a different name for this:
format = bzrdir.
branch = bzrdir.
# Trying to create a wiki branch (at this stage, identical to trunk)
to_ transport = repo.user_ transport. clone(' wiki')
to_ transport = repo._transport .clone( 'wiki') format_ registry. make_bzrdir( 'default' ) BzrDir. create_ branch_ convenience(
to_ transport. base, format=format,
possible_ transports= [to_transport] )
branch. set_append_ revisions_ only(True)
try:
except AttributeError:
# Fixes lp:948483 bzrlib version 2.1 (which is current in debian
# squeeze) uses a different name for this:
format = bzrdir.
branch = bzrdir.
Repeated code is almost always a bad thing. The code to create the branch should be put into a method of it's own, something like this:
def create_ branch_ on_repository( self, repo, branch_name):
to_ transport = repo.user_ transport. clone(branch_ name)
to_ transport = repo._transport .clone( 'wiki') format_ registry. make_bzrdir( 'default' ) BzrDir. create_ branch_ convenience(
to_ transport. base, format=format,
possible_ transports= [to_transport] )
branch. set_append_ revisions_ only(True)
try:
except AttributeError:
# Fixes lp:948483 bzrlib version 2.1 (which is current in debian
# squeeze) uses a different name for this:
format = bzrdir.
branch = bzrdir.
...which leads me to my next point: the name of the method is 'create_ shared_ repository_ for_project' . However, what we're actually doing is creating a shared repository AND some branches. I'd be happier if the 'create_ shared_ repository_ for_project' method only created a shared repository, and calling code then called the new "create_ branch_ on_repository" method as well, in order to create any branches that are needed. One advantage of this is that we can enable or disable the creation of the wiki branch in the web-app code, which is where it belongs. Finally, making methods (and classes, for that matter) as small as possible makes them much easier to test - which leads me nicely to...