Patch #37614
closed
Cleanup app/models/repository/git.rb
Added by Go MAEDA about 2 years ago.
Updated about 2 years ago.
Category:
Code cleanup/refactoring
Description
The attached patches improve the readability of app/models/repository/git.rb.
Files
- Target version set to 5.1.0
Setting the target version to 5.1.0.
The changes should all be equivalent to what's currently there.
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 nil
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.
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.
Holger Just wrote:
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 nil
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.
Thank you for reviewing the patches. The point you made about report_last_commit
method is right. I will commit the patches after removing the change to the method.
- Status changed from New to Closed
- Assignee set to Go MAEDA
Also available in: Atom
PDF