search

minus5 / gofreetds

Go Sql Server database driver.
MIT License
113 stars 48 forks source link

Fixed a reported race condition in conn.go #32

Closed MarkSonghurst closed 8 years ago

MarkSonghurst commented 8 years ago

As reported by the Go race condition checker.

caledhwa commented 8 years ago

Looks like a good idea +1

Did you run gofmt? Looks like a lot of cleanup of spaces.

MarkSonghurst commented 8 years ago

Yes I ran go fmt - I try to make a habit of it :-)

Sent from my iPhone

On 04 Mar 2016, at 18:48, Cale Hoopes notifications@github.com wrote:

Looks like a good idea +1

Did you run gofmt? Looks like a lot of cleanup of spaces.

— Reply to this email directly or view it on GitHub.

caledhwa commented 8 years ago

Good call. I was tempted but didn’t want to make too many updates in my PR that were unrelated. I’d love to see this project get some CI somewhere. However, I haven’t been able to find one that offers a SQL Server service (like TravisCI offers a MongoDB, Postgres, etc). It’d be way cool to get badges such as coverage, and test results, etc. Since it’s all open source it should be free. And we could add a Go Report Card (which would have blessed you for your gofmt update!). Good stuff.

+1 your PR though, just need to get @ianic on it ;-)

On March 4, 2016 at 8:57:41 AM, Mark Songhurst (notifications@github.com) wrote:

Yes I ran go fmt - I try to make a habit of it :-)

Sent from my iPhone

On 04 Mar 2016, at 18:48, Cale Hoopes notifications@github.com wrote:

Looks like a good idea +1

Did you run gofmt? Looks like a lot of cleanup of spaces.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

ianic commented 8 years ago

Thanks Mark.

Did you manage to setup test environment and run gofreetds test locally. We have pretty good test coverage 88.7%.

MarkSonghurst commented 8 years ago

You are welcome. Sorry, I did not run the test suite, I do not have access to a SQL Server I can install Pubs onto. If one could be made available for contributors of this group, that would be really useful?

caledhwa commented 8 years ago

If we are interested... I can look into this

Sent from the 2-6 (Leavenworth, WA 98826)

On Mar 5, 2016, at 06:23, Mark Songhurst notifications@github.com wrote:

You are welcome. Sorry, I did not run the test suite I'm afraid, I do not have access to a SQL Server I can install Pubs onto. If one could be made available for contributors of this group, that would be really useful?

— Reply to this email directly or view it on GitHub.

MarkSonghurst commented 8 years ago

+1 from me - thanks Cale.

Sent from my iPhone

On 05 Mar 2016, at 18:00, Cale Hoopes notifications@github.com wrote:

If we are interested... I can look into this

Sent from the 2-6 (Leavenworth, WA 98826)

On Mar 5, 2016, at 06:23, Mark Songhurst notifications@github.com wrote:

You are welcome. Sorry, I did not run the test suite I'm afraid, I do not have access to a SQL Server I can install Pubs onto. If one could be made available for contributors of this group, that would be really useful?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.