Skip to content

Conversation

@jonasfranz
Copy link
Member

Fix #3784

This is not the most elegant solution but it works.

Add test case

Signed-off-by: Jonas Franz <info@jonasfranz.software>
@codecov-io
Copy link

codecov-io commented May 27, 2018

Codecov Report

Merging #4058 into master will increase coverage by 0.01%.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4058      +/-   ##
==========================================
+ Coverage   19.96%   19.97%   +0.01%     
==========================================
  Files         153      153              
  Lines       30482    30491       +9     
==========================================
+ Hits         6086     6091       +5     
- Misses      23483    23486       +3     
- Partials      913      914       +1
Impacted Files Coverage Δ
modules/util/util.go 29.26% <73.33%> (+7.39%) ⬆️

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 f86f56e...df6d7e3. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 27, 2018
Copy link
Member

@sapk sapk left a comment

Choose a reason for hiding this comment

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

Please use https://golang.org/pkg/net/url/#URL.ResolveReference .
As it is the recommended way to do it.

@jonasfranz
Copy link
Member Author

@sapk If I use URL.ResolveReference, the other test cases fail.

@sapk
Copy link
Member

sapk commented May 28, 2018

@JonasFranzDEV Only relative one are failing https://play.golang.org/p/kk0OHdtTg0i

@sapk
Copy link
Member

sapk commented May 28, 2018

And this could easely be check by u.IsAbs()

jonasfranz and others added 2 commits May 28, 2018 23:18
@bkcsoft bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 28, 2018
@bkcsoft bkcsoft 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 29, 2018
@techknowlogick techknowlogick merged commit 2139c15 into go-gitea:master May 29, 2018
@lunny
Copy link
Member

lunny commented May 29, 2018

@JonasFranzDEV please send back port to release/v1.4

@jonasfranz jonasfranz deleted the bug/fix-md-anchor branch May 29, 2018 08:14
@jonasfranz jonasfranz restored the bug/fix-md-anchor branch May 29, 2018 08:14
@lunny lunny added the backport/done All backports for this PR have been created label May 29, 2018
This was referenced May 30, 2018
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

markdown link anchor issue

6 participants