{"thread":{"id":"42561","subject":"[PATCH] refs: mark the file-local vtable symbols as static","startedAt":"2016-06-08T14:48:02Z","lastAt":"2016-06-16T02:19:49Z","messageCount":3,"participants":["Ramsay Jones","Junio C Hamano","Michael Haggerty"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"288722","messageId":"57583022.2030205@ramsayjones.plus.com","threadId":"42561","inReplyTo":null,"subject":"[PATCH] refs: mark the file-local vtable symbols as static","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-08T14:48:02Z","receivedAt":"2016-06-08T14:48:02Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n\nHi Michael, Junio,\n\nI would normally ask you to squash this into the relevant patch when\nyou next re-roll your 'mh/ref-iterators' branch, but this has already\nbeen merged into next. (I normally have a bit more time ... sorry!).\n\nPerhaps, after the release, when the next branch is re-wound/re-built,\nthis could be squashed into your branch then.\n\nAnyway, after applying this patch, the following symbols are still\n'public but unused':\n\n\t> refs/files-backend.o\t- files_reflog_iterator_begin\n\t> refs/iterator.o\t- is_empty_ref_iterator\n\t> refs/iterator.o\t- merge_ref_iterator_begin\n\nThese all look (potentially) useful for the implementation of\nadditional 'ref-iter' types and look to be part of the _internal_\niterator API - so they should not be marked static. Can you just\nconfirm my interpretation.\n\nThanks.\n\nATB,\nRamsay Jones\n\n refs/files-backend.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a006a65..6213891 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -711,7 +711,7 @@ static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)\n \treturn ITER_DONE;\n }\n \n-struct ref_iterator_vtable cache_ref_iterator_vtable = {\n+static struct ref_iterator_vtable cache_ref_iterator_vtable = {\n \tcache_ref_iterator_advance,\n \tcache_ref_iterator_peel,\n \tcache_ref_iterator_abort\n@@ -1933,7 +1933,7 @@ static int files_ref_iterator_abort(struct ref_iterator *ref_iterator)\n \treturn ok;\n }\n \n-struct ref_iterator_vtable files_ref_iterator_vtable = {\n+static struct ref_iterator_vtable files_ref_iterator_vtable = {\n \tfiles_ref_iterator_advance,\n \tfiles_ref_iterator_peel,\n \tfiles_ref_iterator_abort\n@@ -3354,7 +3354,7 @@ static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator)\n \treturn ok;\n }\n \n-struct ref_iterator_vtable files_reflog_iterator_vtable = {\n+static struct ref_iterator_vtable files_reflog_iterator_vtable = {\n \tfiles_reflog_iterator_advance,\n \tfiles_reflog_iterator_peel,\n \tfiles_reflog_iterator_abort\n-- \n2.8.0\n"},{"id":"288753","messageId":"xmqqfusnldc4.fsf@gitster.mtv.corp.google.com","threadId":"42561","inReplyTo":"57583022.2030205@ramsayjones.plus.com","subject":"Re: [PATCH] refs: mark the file-local vtable symbols as static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T18:55:55Z","receivedAt":"2016-06-08T18:55:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Signed-off-by: Ramsay Jones \n> ---\n>\n> Hi Michael, Junio,\n>\n> I would normally ask you to squash this into the relevant patch when\n> you next re-roll your 'mh/ref-iterators' branch, but this has already\n> been merged into next. (I normally have a bit more time ... sorry!).\n>\n> Perhaps, after the release, when the next branch is re-wound/re-built,\n> this could be squashed into your branch then.\n\nYup, sounds like a plan.\n\n>\n> Anyway, after applying this patch, the following symbols are still\n> 'public but unused':\n>\n> \t> refs/files-backend.o\t- files_reflog_iterator_begin\n> \t> refs/iterator.o\t- is_empty_ref_iterator\n> \t> refs/iterator.o\t- merge_ref_iterator_begin\n>\n> These all look (potentially) useful for the implementation of\n> additional 'ref-iter' types and look to be part of the _internal_\n> iterator API - so they should not be marked static. Can you just\n> confirm my interpretation.\n\nI am not Michael, but FWIW I think that is sensible.\n"},{"id":"288806","messageId":"575963BD.6050302@alum.mit.edu","threadId":"42561","inReplyTo":"xmqqfusnldc4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] refs: mark the file-local vtable symbols as static","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-06-09T12:40:29Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/08/2016 08:55 PM, Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> Signed-off-by: Ramsay Jones \n>> ---\n>>\n>> Hi Michael, Junio,\n>>\n>> I would normally ask you to squash this into the relevant patch when\n>> you next re-roll your 'mh/ref-iterators' branch, but this has already\n>> been merged into next. (I normally have a bit more time ... sorry!).\n>>\n>> Perhaps, after the release, when the next branch is re-wound/re-built,\n>> this could be squashed into your branch then.\n> \n> Yup, sounds like a plan.\n> \n>>\n>> Anyway, after applying this patch, the following symbols are still\n>> 'public but unused':\n>>\n>> \t> refs/files-backend.o\t- files_reflog_iterator_begin\n>> \t> refs/iterator.o\t- is_empty_ref_iterator\n>> \t> refs/iterator.o\t- merge_ref_iterator_begin\n>>\n>> These all look (potentially) useful for the implementation of\n>> additional 'ref-iter' types and look to be part of the _internal_\n>> iterator API - so they should not be marked static. Can you just\n>> confirm my interpretation.\n> \n> I am not Michael, but FWIW I think that is sensible.\n\nI *am* Michael, and I think your changes look good. Thanks for your review.\n\nI've incorporated your changes with some other changes in a re-roll [1]\nin case Junio wants to use it in that form. Please note that two of the\nhunks that you are suggesting apply to \"refs: introduce an iterator\ninterface\" and the third to \"for_each_reflog(): reimplement using\niterators\".\n\nMichael\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/296883\n"}]}