https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292022-08-28T07:09:48ZRedmineRedmine - Patch #37614: Cleanup app/models/repository/git.rbhttps://www.redmine.org/issues/37614?journal_id=1077242022-08-28T07:09:48ZGo MAEDA
<ul><li><strong>File</strong> <a href="/attachments/29610">0006-Cleanup-Repository-Git-clear_extra_info_of_changeset.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/29610/0006-Cleanup-Repository-Git-clear_extra_info_of_changeset.patch">0006-Cleanup-Repository-Git-clear_extra_info_of_changeset.patch</a> added</li></ul> Redmine - Patch #37614: Cleanup app/models/repository/git.rbhttps://www.redmine.org/issues/37614?journal_id=1077612022-09-01T05:19:13ZGo MAEDA
<ul><li><strong>Target version</strong> set to <i>5.1.0</i></li></ul><p>Setting the target version to 5.1.0.</p> Redmine - Patch #37614: Cleanup app/models/repository/git.rbhttps://www.redmine.org/issues/37614?journal_id=1077772022-09-01T19:05:48ZHolger Just
<ul></ul><p>The changes should all be equivalent to what's currently there.</p>
<p>As a tiny personal preference though, I'm not a huge fan of the explicit fetch in your 0005 patch. While it makes it make sit very explicit that we expect a <code>nil</code> here (which I guess is why Rubocop recommends this) but it looks rather in-elegant and verbose, especially considering that we check for the nil just one line below.</p>
<p>Apart from that, it looks good to me. If there is no simple way to shut up Rubocop about the explicit fetch, I would be fine with that too if it helps keep the coding style consistent.</p> Redmine - Patch #37614: Cleanup app/models/repository/git.rbhttps://www.redmine.org/issues/37614?journal_id=1079982022-09-24T04:34:55ZGo MAEDA
<ul></ul><p>Holger Just wrote:</p>
<blockquote>
<p>As a tiny personal preference though, I'm not a huge fan of the explicit fetch in your 0005 patch. While it makes it make sit very explicit that we expect a <code>nil</code> here (which I guess is why Rubocop recommends this) but it looks rather in-elegant and verbose, especially considering that we check for the nil just one line below.</p>
</blockquote>
<p>Thank you for reviewing the patches. The point you made about <code>report_last_commit</code> method is right. I will commit the patches after removing the change to the method.</p> Redmine - Patch #37614: Cleanup app/models/repository/git.rbhttps://www.redmine.org/issues/37614?journal_id=1080082022-09-25T05:08:49ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Closed</i></li><li><strong>Assignee</strong> set to <i>Go MAEDA</i></li></ul><p>Committed the patches.</p>