Skip to content

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented May 8, 2022

When rendering a diff we currently calculate the SHA1 hash of the filename for every row. This PR calculates the hash only once. It may not be measurable but saves some cpu cycles.

@KN4CK3R KN4CK3R added the type/enhancement An improvement of existing functionality label May 8, 2022
@KN4CK3R KN4CK3R added this to the 1.17.0 milestone May 8, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 8, 2022
@wxiaoguang
Copy link
Contributor

After reading the code again, I found that the helper function Sha1 is never used after this improvment, so it's safe to delete it.

I just made two small changes:

  • remove unused Sha1 template helper function
  • use ctx.Data["FileNameHash"] (instead of use ctx.Data["NameHash"])

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 8, 2022
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@9efa471). Click here to learn what that means.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##             main   #19654   +/-   ##
=======================================
  Coverage        ?   47.36%           
=======================================
  Files           ?      956           
  Lines           ?   133417           
  Branches        ?        0           
=======================================
  Hits            ?    63192           
  Misses          ?    62587           
  Partials        ?     7638           
Impacted Files Coverage Δ
modules/templates/helper.go 49.84% <ø> (ø)
routers/web/repo/compare.go 46.95% <0.00%> (ø)
services/gitdiff/gitdiff.go 72.39% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9efa471...7b233f5. Read the comment docs.

@6543 6543 merged commit a9ca4b4 into go-gitea:main May 8, 2022
@6543 6543 added type/refactoring Existing code has been cleaned up. There should be no new functionality. and removed type/enhancement An improvement of existing functionality labels May 8, 2022
@KN4CK3R KN4CK3R deleted the enhancement-sha1 branch May 9, 2022 05:01
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 9, 2022
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Calculate filename hash only once (go-gitea#19654)
  Admin should not delete himself (go-gitea#19423)
  Restore reviewed-on message  (go-gitea#19657)
  Move some helper files out of models (go-gitea#19355)
  Repository level enable package or disable (go-gitea#19323)
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
* Calculate hash only once.

* remove unused Sha1 template helper function, use ctx.Data["FileNameHash"]

* fix unit tests
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants