X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/blobdiff_plain/e3596dbaecb69711616b7bf3b3ef363ee5929293..1bbdc76f9afb9cffe5fb39dea3903dfa3dafbc7e:/doc/todo/headless_git_branches.mdwn diff --git a/doc/todo/headless_git_branches.mdwn b/doc/todo/headless_git_branches.mdwn index 1dd867765..bedf21d0c 100644 --- a/doc/todo/headless_git_branches.mdwn +++ b/doc/todo/headless_git_branches.mdwn @@ -6,7 +6,9 @@ Ikiwiki should really survive being asked to work with a git branch that has no git clone barerepo.git srcdir ikiwiki --rcs=git srcdir destdir -I've fixed this initial construction case, and, based on my testing, I've also fixed the post-update executing on a new master, and ikiwiki.cgi executing on a non-existent master cases. +I've fixed this initial construction case, and, based on my testing, I've +also fixed the post-update executing on a new master, and ikiwiki.cgi +executing on a non-existent master cases. Please commit so my users stop whining at me about having clean branches to push to, the big babies. @@ -14,6 +16,12 @@ Summary: Change three scary loud failure cases related to empty branches into th [[!tag patch]] +> FWIW, [[The_TOVA_Company]] apparently wants this feature (and I hope +> I don't mind that I mention they were willing to pay someone for it, +> but I told them I'd not done any of the work. :) ) +> +> Code review follows, per hunk.. --[[Joey]] +
 diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm
 index cf7fbe9..e5bafcf 100644
@@ -50,6 +58,26 @@ index cf7fbe9..e5bafcf 100644
  
  	return wantarray ? @ci : $ci[0];
  }
+
+ +My concern is that this adds a bit of slowdown (git show-ref is fast, but +It's still extra work) to a very hot code path that is run to eg, +update recentchanges after every change. + +Seems not ideal to do extra work every time to handle a case +that will literally happen a maximum of once in the entire lifecycle of a +wiki (and zero times more typically, since the setup automator puts in a +.gitignore file that works around this problem). + +So as to not just say "no" ... what if it always tried to run git log, +and if it failed (or returned no parsed lines), then it could look +at git show-ref to deduce whether to throw an error or not. +--[[Joey]] + +> Ah, but then git-log would still complain "bad revision 'HEAD'" +> --[[Joey]] + +
 @@ -474,7 +478,10 @@ sub rcs_update () {
  	# Update working directory.
  
@@ -61,7 +89,16 @@ index cf7fbe9..e5bafcf 100644
 +		}
  	}
  }
- 
+
+
+ +Same concern here about extra work. Code path is nearly as hot, being +called on every refresh. Probably could be dealt with similarly as above. + +Also, is there any point in breaking the pull up into a +fetch followed by a merge? --[[Joey]] + +
 @@ -559,7 +566,7 @@ sub rcs_commit_helper (@) {
  	# So we should ignore its exit status (hence run_or_non).
  	if (run_or_non('git', 'commit', '-m', $params{message}, '-q', @opts)) {
@@ -72,3 +109,5 @@ index cf7fbe9..e5bafcf 100644
  	}
  	
 
+ +This seems fine to apply. --[[Joey]]