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

Added support for custom string types in ConvertValue. #623

Merged

Conversation

dsmontoya
Copy link
Contributor

Description

For some reason reflect.String is not considered in the switch statement. This PR add it to a case so custom string types can be converted.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

@@ -0,0 +1,13 @@
package mysql
Copy link
Member

Choose a reason for hiding this comment

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

Please add the license header as follows:

// Go MySQL Driver - A MySQL-Driver for Go's database/sql package
//
// Copyright 2017 The Go-MySQL-Driver Authors. All rights reserved.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at http://mozilla.org/MPL/2.0/.

package mysql

@julienschmidt
Copy link
Member

@dsmontoya this PR requires some changes. Please update your PR when time allows it.

@julienschmidt julienschmidt added this to the v1.4 milestone Oct 27, 2017
@dsmontoya
Copy link
Contributor Author

Sorry for the delay. I completely forgot this PR

Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

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

Looks good to me now.
Let's see why Travis is hanging again...

@julienschmidt
Copy link
Member

It seems like Travis somehow managed to not recognize that there is a new commit.
There's also no option to trigger a check or something like that.
I guess the easiest option would be to make another commit, e.g. by rebasing on the current master (or anything else what causes a change to the git history)

@methane methane closed this Nov 12, 2017
@methane methane reopened this Nov 12, 2017
@methane
Copy link
Member

methane commented Nov 12, 2017

2017-11-13 7 43 42

Maybe, we need to contact to Travis-CI.

@dsmontoya
Copy link
Contributor Author

dsmontoya commented Nov 13, 2017

Just rebased and pushed a new commit and it's still not working.

@julienschmidt
Copy link
Member

@methane where did you find that? I don't see ANY info about this PR on Travis.

@methane
Copy link
Member

methane commented Nov 13, 2017

@julienschmidt
Copy link
Member

Mailed the Travis support. Let's see....

@julienschmidt
Copy link
Member

Reply from the Travis support:

That PR that you mentioned: #623 is coming from an organization called: entropyx. That specific organization is not known to Travis and we do not run PR's coming from unknown organizations.

While this is intended to stop abuse, it does unfortunately cause some issues in situations like these. We are looking into fixing this, as we don't feel that an organization as a whole should have to create a Travis account when working with another that is using Travis CI.

For the time being, a work around would be to have someone in the entropyx organization to create an account in Travis. This should prevent these pull requests from being flagged. If you still encounter issues after that, please don't hesitate to reach out and we'll investigate again.

@dsmontoya
Copy link
Contributor Author

dsmontoya commented Nov 15, 2017

It's done now.

@julienschmidt julienschmidt merged commit 0aa39ff into go-sql-driver:master Nov 15, 2017
@julienschmidt
Copy link
Member

Thanks! (and sorry for the trouble with Travis)

@dsmontoya
Copy link
Contributor Author

No problem! And thank you!

bLamarche413 pushed a commit to bLamarche413/mysql that referenced this pull request Mar 23, 2018
…#623)

* Added support for custom string types.

* Add author name

* Added license header

* Added a newline to force a commit.

* Remove newline.
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.

3 participants