Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calculate filename hash only once #19654

Merged
merged 6 commits into from
May 8, 2022
Merged

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