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

Rename orm_user to account and document as opaque-type #89

Merged
merged 1 commit into from
Jul 13, 2016

Conversation

kitchoi
Copy link
Contributor

@kitchoi kitchoi commented Jul 12, 2016

Fix #67

@codecov-io
Copy link

Current coverage is 79.13%

Merging #89 into master will decrease coverage by 0.02%

@@             master        #89   diff @@
==========================================
  Files            27         27          
  Lines           907        906     -1   
  Methods           0          0          
  Messages          0          0          
  Branches         83         83          
==========================================
- Hits            718        717     -1   
  Misses          163        163          
  Partials         26         26          

Powered by Codecov. Last updated by cbbed1d...e8b5d40

"""

@abstractmethod
def get_apps_for_user(self, user):
""" Return an iterable of ApplicationConfig for a given User
def get_apps_for_user(self, account):
Copy link
Contributor

Choose a reason for hiding this comment

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

At this point I wonder if we should use get_apps_for_account instead.

Copy link
Contributor Author

@kitchoi kitchoi Jul 13, 2016

Choose a reason for hiding this comment

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

In my previous branch I did that, but think it might be too trivial that it is not worth breaking the public API that is in a public release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that anyone expects API consistency before 1.0...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't mind breaking it at all if we all agree to it.

@kitchoi
Copy link
Contributor Author

kitchoi commented Jul 13, 2016

@sbo Can I merge? I needed this for #71

@stefanoborini stefanoborini merged commit fb1bf2c into master Jul 13, 2016
@stefanoborini stefanoborini deleted the 67-orm-user branch July 13, 2016 14:17
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