Patch #37614

Cleanup app/models/repository/git.rb

Added by Go MAEDA about 1 month ago. Updated 4 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Code cleanup/refactoring
Target version:5.1.0

Description

The attached patches improve the readability of app/models/repository/git.rb.

0001-Fix-RuboCop-offense-Rails-Blank.patch Magnifier (1.08 KB) Go MAEDA, 2022-08-28 07:15

0002-Fix-RuboCop-offense-Style-SymbolProc.patch Magnifier (1.73 KB) Go MAEDA, 2022-08-28 07:15

0003-Fix-RuboCop-offense-Style-IfUnlessModifier.patch Magnifier (1.57 KB) Go MAEDA, 2022-08-28 07:15

0004-Fix-RuboCop-offense-Style-GuardClause.patch Magnifier (924 Bytes) Go MAEDA, 2022-08-28 07:15

0005-Use-the-safe-navigation-operator-instead-of-checking.patch Magnifier (1.5 KB) Go MAEDA, 2022-08-28 07:15

0006-Cleanup-Repository-Git-clear_extra_info_of_changeset.patch Magnifier (857 Bytes) Go MAEDA, 2022-08-28 09:09

Associated revisions

Revision 21840
Added by Go MAEDA 4 days ago

Fix RuboCop offense Rails/Blank (#37614).

Revision 21841
Added by Go MAEDA 4 days ago

Fix RuboCop offense Style/SymbolProc (#37614).

Revision 21842
Added by Go MAEDA 4 days ago

Fix RuboCop offense Style/IfUnlessModifier (#37614).

Revision 21843
Added by Go MAEDA 4 days ago

Fix RuboCop offense Style/GuardClause (#37614).

Revision 21844
Added by Go MAEDA 4 days ago

Use the safe navigation operator instead of checking if an object is nil (#37614).

Patch by Go MAEDA.

Revision 21845
Added by Go MAEDA 4 days ago

Cleanup Repository::Git#clear_extra_info_of_changesets (#37614).

Patch by Go MAEDA.

Revision 21846
Added by Go MAEDA 4 days ago

Regenerate .rubocop_todo.yml (#37614).

History

#2 Updated by Go MAEDA 28 days ago

  • Target version set to 5.1.0

Setting the target version to 5.1.0.

#3 Updated by Holger Just 27 days ago

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.

#4 Updated by Go MAEDA 5 days ago

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.

#5 Updated by Go MAEDA 4 days ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed the patches.

Also available in: Atom PDF