Skip to content

Support use nvarchar for all varchar columns when using mssql#12269

Merged
lunny merged 4 commits into
go-gitea:masterfrom
lunny:lunny/config_mssql_varchar
Jul 21, 2020
Merged

Support use nvarchar for all varchar columns when using mssql#12269
lunny merged 4 commits into
go-gitea:masterfrom
lunny:lunny/config_mssql_varchar

Conversation

@lunny
Copy link
Copy Markdown
Member

@lunny lunny commented Jul 18, 2020

Should fix #12094

@lunny lunny added the type/enhancement An improvement of existing functionality label Jul 18, 2020
@lunny lunny added this to the 1.13.0 milestone Jul 18, 2020
@lafriks
Copy link
Copy Markdown
Member

lafriks commented Jul 18, 2020

Why not just always set nvarchar?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 18, 2020
@lunny
Copy link
Copy Markdown
Member Author

lunny commented Jul 18, 2020

Why not just always set nvarchar?

I think that will break the previous installation. To keep compitable, the default value is varchar.

@lafriks
Copy link
Copy Markdown
Member

lafriks commented Jul 18, 2020

Why it would break? It would just set nvarchar for new columns

@lunny
Copy link
Copy Markdown
Member Author

lunny commented Jul 19, 2020

Why it would break? It would just set nvarchar for new columns

done.

@lunny lunny mentioned this pull request Jul 19, 2020
Closed
7 tasks
@lafriks
Copy link
Copy Markdown
Member

lafriks commented Jul 19, 2020 •
edited
Loading

I still don't know if setting is needed. Why would anyone would want to use varchar instead of nvarchar as then any unicode character will be broken?

@lunny
Copy link
Copy Markdown
Member Author

lunny commented Jul 19, 2020

@lafriks for those who use latin characters only and want less disk usage.

@lafriks
Copy link
Copy Markdown
Member

lafriks commented Jul 19, 2020

Yeah but emoji chars are also unicode. Also I don't think Gitea database size is what would be such a problem, git repos are usually that takes up most space anyway

@lafriks
Copy link
Copy Markdown
Member

lafriks commented Jul 19, 2020

imho we are already have way too many settings 😬

@lunny
Copy link
Copy Markdown
Member Author

lunny commented Jul 20, 2020

imho we are already have way too many settings 😬

Reasonable. Done.

@lunny lunny force-pushed the lunny/config_mssql_varchar branch from 2f41477 to 5fe2453 Compare July 20, 2020 04:45
@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 Jul 20, 2020
@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 Jul 21, 2020
@lunny lunny merged commit 4563108 into go-gitea:master Jul 21, 2020
@lunny lunny deleted the lunny/config_mssql_varchar branch July 21, 2020 12:34
@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.

Reviewers

@lafriks lafriks lafriks approved these changes

+1 more reviewer

@jaqra jaqra jaqra approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality

Projects

None yet

Milestone

1.13.0

Development

Successfully merging this pull request may close these issues.

Arabic text shows as question marks (?)

4 participants

@lunny @lafriks @jaqra @GiteaBot