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

Flag as an abstract base class #218

Closed
wants to merge 20 commits into from
Closed

Conversation

cleder
Copy link
Member

@cleder cleder commented Jun 4, 2016

No description provided.

@coveralls
Copy link

coveralls commented Jun 4, 2016

Coverage Status

Changes Unknown when pulling 5eea5c6 on PrimarySite:master into * on jsocol:master*.

@FlogFr
Copy link

FlogFr commented Jun 4, 2016

Excellent work/contribution!
I tried to think twice and do a PR on possible default Flag model inside django-waffle, and I think you're right on the explicit over implicit. It would be weird to have a default Flag model which would be not represented in the migrations and then the migrations would need to be generated in the django-waffle applications...

Thanks again :)

@cleder
Copy link
Member Author

cleder commented Jun 29, 2016

@jsocol any thoughts about this PR?

@jsocol
Copy link
Collaborator

jsocol commented Jun 29, 2016

I haven't had a lot of time to dig into this one yet, but I have a hard time seeing myself merging it as-is. I think there are some good ideas here, but they're wound up with a bunch of other changes.

I've been underwater with work this month, but I'm going to try to find some time to keep moving on 0.12 over the holiday weekend. I don't expect abstract base models to be part of that release.

@coveralls
Copy link

coveralls commented Jan 4, 2017

Coverage Status

Changes Unknown when pulling 4155731 on PrimarySite:master into ** on jsocol:master**.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 4, 2017

Coverage Status

Changes Unknown when pulling 4155731 on PrimarySite:master into ** on jsocol:master**.

@codecov-io
Copy link

codecov-io commented Jan 4, 2017

Current coverage is 93.58% (diff: 100%)

No coverage report found for master at 7e3ee1b.

Powered by Codecov. Last update 7e3ee1b...69052e5

Marcin Rutkowski and others added 2 commits January 4, 2017 12:50
…support-COM-1438

feature/add-custom-flag-model-support-COM-1438
@coveralls
Copy link

coveralls commented Jan 13, 2017

Coverage Status

Changes Unknown when pulling 69052e5 on PrimarySite:master into ** on jsocol:master**.

2 similar comments
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 69052e5 on PrimarySite:master into ** on jsocol:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 69052e5 on PrimarySite:master into ** on jsocol:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 69052e5 on PrimarySite:master into ** on jsocol:master**.

2 similar comments
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 69052e5 on PrimarySite:master into ** on jsocol:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 69052e5 on PrimarySite:master into ** on jsocol:master**.

@koliber
Copy link
Contributor

koliber commented Apr 10, 2017

I really like the way Django implements custom user models. I recently had an opportunity to ask Russell Keith-Magee, the author of Django's custom user models, on what he would recommend. He explained how that was done and pointed out some pitfalls.

I took his advice and build custom flag models emulating the way Django did custom user models: #243 .

@stale stale bot added the stale label Aug 26, 2017
@stale
Copy link

stale bot commented Aug 26, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot closed this Sep 9, 2017
@stale
Copy link

stale bot commented Sep 9, 2017

This issue has been closed as stale because it has not had any recent activity. Please feel free to re-open if the issue is still relevant. Thank you for your contributions!

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

Successfully merging this pull request may close these issues.

None yet

7 participants