Skip to content

Conversation

@hube
Copy link
Contributor

@hube hube commented Jan 28, 2014

I merged #132 with the latest master. I couldn't figure out how to submit these changes to that PR, so I created this one instead.

This fixes Issue #83

example:
@table(name = "Items", id = BaseColumns._ID)
public class Item extends Model {...}

Conflicts:
	src/com/activeandroid/Model.java
	src/com/activeandroid/TableInfo.java
@joshuapinter
Copy link
Contributor

@SeanPONeil Can we get this pull request merged in, seems like a key feature to be able to set the Id column. What's nice is that it defaults to the regular "Id" unless you specifically set it, so it shouldn't cause any conflict.

SeanPONeil added a commit that referenced this pull request Mar 2, 2014
Add method in Table annotation to set column Id name.
@SeanPONeil SeanPONeil merged commit 6ab1e05 into pardom-zz:master Mar 2, 2014
@SeanPONeil
Copy link
Contributor

I should have waited for the Travis build to pass before merging :/. Any idea why this is failing?

@joshuapinter
Copy link
Contributor

I'll take a look.

joshuapinter added a commit to joshuapinter/ActiveAndroid that referenced this pull request Mar 3, 2014
Unique Groups and Index Groups caused a conflict with allowing for a custom Id column name.

This was discovered in Pull Request pardom-zz#175.
@joshuapinter
Copy link
Contributor

I created another pull request to fix the issue raised with this pull request (is there a better way to do this?). If you merge that PR in the tests run swimmingly once again.

On a further note, it'd be great to require tests for this new functionality. The issue was due to the Index Groups and Unique Groups not being able to handle the custom Id column naming. I was able to work around it but without tests for those pieces I can't be certain they weren't affected.

SeanPONeil added a commit that referenced this pull request Mar 3, 2014
Fixes Issue from Pull Request #175.
@hube
Copy link
Contributor Author

hube commented Mar 5, 2014

Thanks for merging! Sorry about the broken build, I didn't run the tests after I merged in the old PR :(

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