Skip to content

Conversation

mrsdizzie
Copy link
Member

@mrsdizzie mrsdizzie commented May 3, 2019

Don't run summary through render/post process on certain list views where it is already expected to be a link to the individual commit. This avoids inconsistent UI in some cases.

Fixes #6809

Don't run summary through render/post process to avoid it generating
links and breaking certain views where the summary is already expected
to be a link to the commit. For consistancy, disable processing of
summary in all locations.

Fixes go-gitea#6809
@codecov-io
Copy link

codecov-io commented May 3, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@9148e45). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6842   +/-   ##
=========================================
  Coverage          ?   41.14%           
=========================================
  Files             ?      425           
  Lines             ?    58484           
  Branches          ?        0           
=========================================
  Hits              ?    24061           
  Misses            ?    31238           
  Partials          ?     3185

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 9148e45...6f57f88. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 3, 2019
On second thought, the title is often the only place a pull request
number will exists so do process it on the individual diff page. This
fixes the list view and still gives easy access to the PR link
@mrsdizzie
Copy link
Member Author

Also up for feedback if keeping certain auto generated links in the summary title is something that is a very desired feature.

@GiteaBot GiteaBot 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 5, 2019
@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 5, 2019
@mrsdizzie mrsdizzie mentioned this pull request May 5, 2019
8 tasks
@techknowlogick techknowlogick merged commit 00b8838 into go-gitea:master May 5, 2019
@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

As @lunny says in the underlying issue, this should probably be backported.

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/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Submit the history html 404 pushed by client
7 participants