Derrick Stolee <derrickstolee@xxxxxxxxxx> writes: >>> + for_each_decoration(commit, append_decoration, &ctx); >> >> The function for_each_decoration() that does not take decoration but >> a commit felt iffy,... >> ... >> for_each_decoration(get_name_decoration(&commit->object), >> do_whatever_they_need_to_do, &ctx); >> >> so, we may want to revisit this after we finish reading the series >> through. Making the iterator take name_decoration does not look >> too bad. > > You are right that this would work, but it's a bit messier. We > could go this route to avoid the duplicate get_name_decoration() > calls in format_decorations_extended(). Having a convenience function that takes a commit and let you walk over its decoration is OK, but calling it for_each_decoration() bothers me somewhat (which is where my comment started from after all). Hopefully it will stay unambiguous, and it will stay to be unnecessary to name it for_each_decoration_for_commit(), as I do not think we'd add "decoration" to things that are not commits (or if we added one, we probably will call it differently from "decoration"). So, OK.
