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

Allow for fixed-size binary primary key #217

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow for fixed-size binary primary key #217

wants to merge 2 commits into from

Conversation

devinivy
Copy link
Contributor

@devinivy devinivy commented May 9, 2015

This allows for a user to define a model with a binary type primary key. Resolves #215 . This should be tested before being merged.

CC: @pakalex @dmarcelino @tjwebb

@dmarcelino
Copy link
Member

Looks good! Any chance of adding a failing test for this?

@devinivy
Copy link
Contributor Author

devinivy commented May 9, 2015

Sure– would probably be a unit test in this repo, though. I don't know that every adapter could possibly support a binary primary key type. What's the simplest way to get unit tests working alongside the load and integration runners?

Edit
Or, maybe it could be part of the SQL interface?

@dmarcelino
Copy link
Member

Yes, I agree with the unit test. I would say the simplest way is a Makefile like the one in sails-postgresql: https://github.com/balderdashy/sails-postgresql/blob/master/Makefile

And then change the package.json accordingly to:

  "scripts": {
    "test": "make test"
  },

@particlebanana
Copy link
Contributor

Unable to be merged now but seems like it's still a valid issue. I'm so ready for migrations to be their own thing but if this is still an issue for people we should re-submit this.

@camsjams
Copy link

Will this ever be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants