Skip to content

Conversation

lafriks
Copy link
Member

@lafriks lafriks commented Mar 6, 2018

Fixes #3624

@lafriks lafriks added this to the 1.5.0 milestone Mar 6, 2018
@codecov-io
Copy link

codecov-io commented Mar 6, 2018

Codecov Report

Merging #3638 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #3638 +/- ## ========================================== - Coverage 35.92% 35.89% -0.04%  ========================================== Files 286 286 Lines 41257 41286 +29 ========================================== - Hits 14823 14820 -3  - Misses 24255 24286 +31  - Partials 2179 2180 +1
Impacted Files Coverage Δ
models/migrations/v56.go 0% <0%> (ø) ⬆️
models/migrations/migrations.go 2.62% <0%> (-0.27%) ⬇️
modules/process/manager.go 76.81% <0%> (-4.35%) ⬇️

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 4009c24...8d92ba4. Read the comment docs.

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

Choose a reason for hiding this comment

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

How about use a transaction here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Alter table is not transactional but I can add it

@lafriks
Copy link
Member Author

lafriks commented Mar 7, 2018

@lunny done

@lafriks lafriks force-pushed the fix/mssql_drop_column branch from 5ffd16f to 8d92ba4 Compare March 7, 2018 06:24
@tboerger tboerger 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 Mar 7, 2018
@lafriks
Copy link
Member Author

lafriks commented Mar 7, 2018

MSSQL alter table is actually transaction so I added transaction as well

@tboerger tboerger 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 Mar 7, 2018
@appleboy appleboy merged commit dc4afef into go-gitea:master Mar 7, 2018
@lafriks lafriks deleted the fix/mssql_drop_column branch March 7, 2018 07:10
lafriks added a commit to lafriks-fork/gitea that referenced this pull request Mar 7, 2018
* Fix column removal in MSSQL * Use xorm session in MSSQL drop column operations * Add transaction as MSSQL alter table is transactional
@lafriks lafriks added the backport/done All backports for this PR have been created label Mar 7, 2018
lafriks added a commit that referenced this pull request Mar 7, 2018
* Fix column removal in MSSQL * Use xorm session in MSSQL drop column operations * Add transaction as MSSQL alter table is transactional
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
project-mirrors-bot-tu bot pushed a commit to project-mirrors/forgejo-as-gitea-fork that referenced this pull request Jan 23, 2025
Fix go-gitea#3638 This is a manual Forgejo-specific version of the Gitea PR go-gitea#30862. The weekly Forgejo PR go-gitea#3772 could not cherry-pick this commit due to conflicts (eg subsequent CodeSpell changes). Only occurs with Webkit in Safari over eg `http://192..`. (not localhost). See https://webkit.org/blog/10855/async-clipboard-api/ --- **Before** ![Before.jpg](/attachments/c570d030-fcce-48ea-ac96-06b624541c7b) **After** ![After.jpg](/attachments/1a9132ab-f7f3-43a5-b3ea-37b6f2b671c4) Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3805 Reviewed-by: Otto <otto@codeberg.org> Co-authored-by: David Davies-Payne <d2p@me.com> Co-committed-by: David Davies-Payne <d2p@me.com> (cherry picked from commit e4c3c03)
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

5 participants