{"comment":{"url":"https://api.github.com/repos/jenkinsci/git-plugin/comments/17743775","html_url":"https://github.com/jenkinsci/git-plugin/commit/0e74daca84c2f0cf75db9243d4d54ffa10abec24#commitcomment-17743775","id":17743775,"user":{"login":"MarkEWaite","id":156685,"avatar_url":"https://avatars.githubusercontent.com/u/156685?v=3","gravatar_id":"","url":"https://api.github.com/users/MarkEWaite","html_url":"https://github.com/MarkEWaite","followers_url":"https://api.github.com/users/MarkEWaite/followers","following_url":"https://api.github.com/users/MarkEWaite/following{/other_user}","gists_url":"https://api.github.com/users/MarkEWaite/gists{/gist_id}","starred_url":"https://api.github.com/users/MarkEWaite/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MarkEWaite/subscriptions","organizations_url":"https://api.github.com/users/MarkEWaite/orgs","repos_url":"https://api.github.com/users/MarkEWaite/repos","events_url":"https://api.github.com/users/MarkEWaite/events{/privacy}","received_events_url":"https://api.github.com/users/MarkEWaite/received_events","type":"User","site_admin":false},"position":null,"line":null,"path":null,"commit_id":"0e74daca84c2f0cf75db9243d4d54ffa10abec24","created_at":"2016-06-04T22:19:54Z","updated_at":"2016-06-04T22:19:54Z","body":"Clearly I didn't understand that code well enough to make any change to it.  My apologies for that.\r\n\r\nI am now concerned that the current code on the master branch https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java#L95 is not doing what was intended.\r\n\r\nCould you review the code as it currently exists on the master branch to assure that it is still behaving reasonably?  I'll then attempt to update the method comment to express that getIndexOfPath is returning a \r\n\r\nThe code seems to determine the insertion point by counting the number of items where compareTo reports the passed in Path is \"less than\" the Path in the list.  That explains why it must traverse the entire list every time, rather than immediately returning when the first matching entry is found.  I mistakenly assumed \"getIndexOfPath\" meant that it would return the location of the passed Path in the changeset.\r\n\r\nI think that also means that if the list is not sorted, and if a Path entry occurs twice in the list, then getIndexOfPath may return a number which is not the location of the matching Path in the list.  Since the GitHubWeb and GitList implementations both include a comment \"by appending the index of the pathname in the changeset\", I'm concerned that the getIndexOfPath method may not be doing what you expected it to do when you wrote it.\r\n\r\nMy example of the hypothetical contents of the path list:\r\n* Path-B\r\n* Path-A\r\n* Path-C\r\n* Path-D\r\n* Path-A\r\n* Path-A\r\n* Path-A\r\n* Path-C\r\n* Path-A\r\n\r\nIf I now search that list for Path-C, I think getIndexOfPath will return 6, since there are 6 items in the list that are less than Path-C.  If index 6 is used to read from that list, it will return Path-A rather than Path-C.  On the positive side, as far as I can tell from reading the code, no code is actually using getIndexOfPath to read from the list.  The return value seems to always be used to assemble a URL, not to read from the list."}}
{"actor":{"display_login":"MarkEWaite"}}