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

Move optimized ucs22str function to 64bit only #43

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

shueybubbles
Copy link
Collaborator

Fixes #39
I also moved some test code to 64bit-only as it either doesn't compile for 32bit or fails during the test run. We aren't going to spend much time debugging such failures since they are longstanding at this point. I added a 32bit test run to the appveyor build to help prevent future 32bit breaks.

Copy link

@apoorvdeshmukh apoorvdeshmukh left a comment

Choose a reason for hiding this comment

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

Is there any workaround for appveyor test failures?
If the -race option is not supported, should we disable this mode?

@shueybubbles
Copy link
Collaborator Author

the -race parameter needs to become part of the matrix in the yml


In reply to: 1057622190

@codecov-commenter
Copy link

Codecov Report

Merging #43 (0f2e818) into main (c33ed63) will increase coverage by 0.53%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
+ Coverage   72.03%   72.56%   +0.53%     
==========================================
  Files          23       25       +2     
  Lines        5604     5596       -8     
==========================================
+ Hits         4037     4061      +24     
+ Misses       1319     1297      -22     
+ Partials      248      238      -10     
Impacted Files Coverage Δ
tds.go 68.58% <ø> (-2.56%) ⬇️
ucs22str.go 100.00% <100.00%> (ø)
ucs22str_386.go 100.00% <100.00%> (ø)
mssql_go110pre.go
mssql_go118.go 100.00% <0.00%> (ø)
token.go 65.02% <0.00%> (+0.14%) ⬆️
mssql.go 86.24% <0.00%> (+0.81%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@shueybubbles shueybubbles merged commit 63479d5 into main Aug 4, 2022
@shueybubbles shueybubbles deleted the shueybubbles/32bit branch August 4, 2022 14:10
@ondrejsika
Copy link

Why?

@shueybubbles
Copy link
Collaborator Author

Why?

Because the original PR author who wrote the optimized function didn't build or test it for 32bit and it was just our quickest option to unblock 32bit consumers. I'm open to someone revisiting the optimized function to make it 32bit compatible.

ondrejsika added a commit to ondrejsika/fork-go-mssqldb-32bit-for-slu that referenced this pull request Feb 3, 2023
ondrejsika added a commit to ondrejsika/fork-go-mssqldb-32bit-for-slu that referenced this pull request Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recent changes to tds.go mean compilation on GOARCH=386 is no longer possible
4 participants