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

Merge fixes for issues #5, #6, #7, #9, #14, #15, #17 #13

Closed
wants to merge 14 commits into from

Conversation

thenewguy
Copy link
Contributor

I merged my commits for issues #5, #6, #7, #9, #14, #15, #17 into one pull request because there were merge conflicts

@thenewguy
Copy link
Contributor Author

e57e21d is a backwards compatible fix for #14

@thenewguy thenewguy changed the title Merge fixes for 5, 6, 7, 9 Merge fixes for 5, 6, 7, 9, 14 Sep 7, 2015
@thenewguy thenewguy changed the title Merge fixes for 5, 6, 7, 9, 14 Merge fixes for issues #5, #6, #7, #9, #14 Sep 7, 2015
@thenewguy thenewguy changed the title Merge fixes for issues #5, #6, #7, #9, #14 Merge fixes for issues #5, #6, #7, #9, #14, #15 Sep 7, 2015
@thenewguy thenewguy changed the title Merge fixes for issues #5, #6, #7, #9, #14, #15 Merge fixes for issues #5, #6, #7, #9, #14, #15, #17 Sep 7, 2015
@nielsvanoch
Copy link
Member

Thanks for the hard work! I'm going to look into all of your pull requests this weekend.

@thenewguy
Copy link
Contributor Author

Sounds good. There is one issue I am not sure how to go about fixing without knowing more about the projects compatibility requirements... it doesn't feel right to expose an AutoID field value to the client so it would be very beneficial to use UUID primary keys for the Lock model to allow their use in the front end.

Because of the nature of locks and how the interaction interface is written, I don't think it would be very disruptive to migrate from an AutoField id to a UUIDField id.

Would you be opposed to changing the primary key field type and the minimum supported version of django after a version bump?

@nielsvanoch
Copy link
Member

That's something I would be ok with, and our requirements allow for it. Feel free to make a proposal!

I merged all of your changes into version 1.1.0, together with some additional changes we had pending. It required a bit of merge magic, but this pull request is in there.

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

Successfully merging this pull request may close these issues.

3 participants