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

Support shortened commit SHAs in URLs (#6450) #6452

Closed
wants to merge 2 commits into from

Conversation

jeblair
Copy link
Contributor

@jeblair jeblair commented Mar 27, 2019

This supports using a git SHA of any length between 7 and 40
characters in URLs (e.g. /src/commit/SHA/...). Previously
only commit SHAs of the full 40 character length were supported.

Signed-off-by: James E. Blair jeblair@redhat.com

Copy link

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 27, 2019
@techknowlogick techknowlogick added this to the 1.9.0 milestone Mar 27, 2019
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Mar 27, 2019
@adelowo
Copy link
Member

adelowo commented Mar 27, 2019

Can we support the shortened sha1 but redirect to the full actual one ?

This supports using a git SHA of any length between 7 and 40
characters in URLs (e.g. /src/commit/SHA/...).  Previously
only commit SHAs of the full 40 character length were supported.

The RepoRefAny ref type is used in one place, in the
/api/v1/user/repo/raw API endpoint where it is used to guess whether
the remainder of the path is a ref name followed by a file path
or merely a file path.  There is no good way to guess whether
a shortened SHA is intended in that circumstance (e.g.,
/raw/beefcafe/README.txt could be /README.txt in the beefcafe{..32}
commit, or /beefcafe/README.txt on the master branch).  For this
case, we don't support shortened SHAs and only match on the full
one.

Signed-off-by: James E. Blair <jeblair@redhat.com>
@codecov-io
Copy link

Codecov Report

Merging #6452 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6452      +/-   ##
==========================================
+ Coverage   39.39%   39.41%   +0.01%     
==========================================
  Files         393      393              
  Lines       53271    53277       +6     
==========================================
+ Hits        20988    20998      +10     
+ Misses      29297    29292       -5     
- Partials     2986     2987       +1
Impacted Files Coverage Δ
modules/context/repo.go 58.9% <100%> (+0.5%) ⬆️
models/repo_list.go 66.84% <0%> (-1.06%) ⬇️
routers/repo/view.go 42.07% <0%> (+0.99%) ⬆️
models/unit.go 14.28% <0%> (+14.28%) ⬆️

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 2019983...341f624. Read the comment docs.

@jeblair
Copy link
Contributor Author

jeblair commented Mar 28, 2019

@adelowo Good question -- that also came up in the issue #6450; I responded there to try to collect that conversation in one place.

@stale
Copy link

stale bot commented May 27, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label May 27, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label May 28, 2019
@stale stale bot removed the issue/stale label May 28, 2019
@techknowlogick techknowlogick modified the milestones: 1.9.0, 1.10.0 Jun 17, 2019
@techknowlogick techknowlogick modified the milestones: 1.10.0, 1.11.0 Sep 3, 2019
@@ -491,8 +491,10 @@ const (
RepoRefBranch
// RepoRefTag tag
RepoRefTag
// RepoRefCommit commit
// RepoRefCommit short or long commit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add a value in the middle of the enums.

@lunny lunny modified the milestones: 1.11.0, 1.12.0 Dec 14, 2019
@CirnoT
Copy link
Contributor

CirnoT commented Apr 29, 2020

Can we support the shortened sha1 but redirect to the full actual one ?

We don't do that for any other endpoint that allows shortened hashes currently, so if we would want to start doing that, wouldn't it be better for another PR that updates all of them to such behavior?

@6543
Copy link
Member

6543 commented Nov 24, 2020

as of my understanding still needed for golang/go#39019 ?

@lafriks lafriks removed this from the 1.14.0 milestone Nov 25, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants