On 02/28/16 07:02, John Keeping wrote:
On Fri, Feb 26, 2016 at 02:58:58PM -0600, Tim Nordell wrote:
The internal logic has been restructured so that there is a "walking"
routine that filters the repo list based on the visible criteria, and
subsequently calls a given callback for each repo found. Additionally,
split out generating a table line for a given repo, and a table line
for a given section. This makes this more loosely coupled and allows
reuse of more logic for changes to the way the repo list is displayed.
This patch does not make any functional changes to the system and is
a reorganization of the existing logic only.
There's a lot going on in this patch that makes it very hard to review.
It would be easier if it were broken down into:
extract struct repolist_ctx
extract the new generate_repolist() function
add repolist_walk_visible()
Thanks for the initial glance and suggestion. I'll break it apart along
these lines.
It would also be helpful to mention in the commit message why
repolist_walk_visible() is needed: in a following patch we will be
adding a variant that prints only section headers.
Noted.
There are also various style problems here, but they're similar to
earlier patches and the Linux kernel's scripts/checkpatch.pl script
catches them.
Yeah, I didn't realize that CGIT was following the kernel's style guide.
I looked in CGIT's folder, but I didn't expect it to be following
documentation inside the submodule. I'll grab the kernel's style
checker and vet the changes against that.
- Tim
_______________________________________________
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
