shilow/boar

Possible refactoring: Repo.__get_repo_version() and LATEST_REPO_FORMAT

Closed this issue · 2 comments

I feel that __get_repo_version() could be refactored, the part checking for 
repository structure should be separated to another function, and REPO_DIRS_V0, 
REPO_DIRS_V1, REPO_DIRS_V2 should be in a list for such function.

looks_like_repo() is redundant because it's the same as checking for version 0.

Original issue reported on code.google.com by uts...@gmail.com on 5 Mar 2012 at 2:23

I'm not so sure about this. There are some subtle differences here. For 
instance, the difference between looks_like_repo() and detecting a v0 repo is 
the difference between "There is no repository at the given path" and "Sorry, 
your repo is corrupted and needs to be thrown out". So even if there is a lot 
in common between these functions, they serve different purposes. And I think 
that is made more clear by having duplicate code in them, since that does not 
falsely give the impression that they always will do the same thing (even 
though that might be the case today).

There is certainly room for refactorings in some parts of the code, but I'd 
need a bit more concrete proposal (like a patch) to decide on an issue like 
this.

Original comment by ekb...@gmail.com on 8 Mar 2012 at 9:30

  • Changed state: WontFix
  • Added labels: Type-Review
  • Removed labels: Type-Defect
I guess you're right. Doesn't look like there'll be a lot of versions anyway so 
refactoring seems to be YAGNI in this case.

Original comment by uts...@gmail.com on 8 Mar 2012 at 9:56