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

Comments on review should have the same sha #13448

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Nov 6, 2020

Restore #13217

…s page

This happened because the comment took the latest commitID as its base instead of the
reviewID that it was replying to.

There was also no way of creating an already outdated comment - and a
reply to a review on an outdated line should be outdated.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added this to the 1.14.0 milestone Nov 6, 2020
@zeripath
Copy link
Contributor Author

zeripath commented Nov 6, 2020

Whilst the migration was broken the correct solution should have been to no-op the migration not revert the whole commit.

In any case hopefully this fixes the migration.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 6, 2020
Signed-off-by: Andrew Thornton <art27@cantab.net>
…com:zeripath/gitea into comments-on-review-should-have-the-same-sha
@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 Nov 6, 2020
@lunny
Copy link
Member

lunny commented Nov 7, 2020

CI randomly failed but may not relate to this.

@zeripath
Copy link
Contributor Author

zeripath commented Nov 7, 2020

Unfortunately the mssql code is incorrect

@zeripath
Copy link
Contributor Author

zeripath commented Nov 7, 2020

mssql: Only one expression can be specified in the select list when the subquery is not introduced with EXISTS.

models/migrations/v158.go Outdated Show resolved Hide resolved
zeripath and others added 9 commits November 7, 2020 18:45
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@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 Nov 9, 2020
@techknowlogick
Copy link
Member

🚀

@techknowlogick techknowlogick merged commit b091c99 into go-gitea:master Nov 9, 2020
@zeripath zeripath deleted the comments-on-review-should-have-the-same-sha branch November 9, 2020 07:02
6543 added a commit to 6543-forks/gitea that referenced this pull request Nov 9, 2020
@jpraet
Copy link
Member

jpraet commented Dec 7, 2020

This may have caused a regression in 1.13 #13683. Could someone have a look at that?

@go-gitea go-gitea locked and limited conversation to collaborators Dec 14, 2020
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.

None yet

6 participants