-
-
Notifications
You must be signed in to change notification settings - Fork 78
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
feat(StateLog): Migrated state log from integerfield to charfield #97
base: master
Are you sure you want to change the base?
Conversation
Tests failed, however I'm unsure why |
It's a flake8 (linter) error. https://travis-ci.org/gizmag/django-fsm-log/jobs/562015145#L507
|
Sorry, I realize I wasn't clear. I don't understand how what I did caused
it to fail as I don't remember touching this part of the codebase.
I'm wondering if this is a known error, especially as looking at that line
I see #noqa:F811 which makes me think the code has changed to F401 recently
…On Tue, 23 Jul 2019 at 14:23, Tyson Clugg ***@***.***> wrote:
Tests failed, however I'm unsure why
It's a flake8 (linter) error.
https://travis-ci.org/gizmag/django-fsm-log/jobs/562015145#L507
django_fsm_log/conf.py:1:1: F401 'django.conf.settings' imported but unused
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#97>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ALEDBXGYYD5G5WN3EYOFBGTQA4A5LANCNFSM4IFWPAJA>
.
--
Josh Harwood
Backend Developer
Stable Group Ltd
Email: [email protected]
Website: stableprice.com <https://stableprice.com/>
Office Address: 3 Whitehall Ct, London, SW1A 2EL, UK
Company Twitter: https://twitter.com/stableprice>
Company LinkedIn: https://www.linkedin.com/company/18252297/
|
), | ||
obj=field, | ||
id='django_fsm_log.W001', | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this warning absolutely necessary? I'm assuming it relates to https://code.djangoproject.com/ticket/25012 or similar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically yeah, one of the only solid bits of contention in the issue where this was discussed was surrounding the use of the field by user based code which implicitly expected an integer.
I am also aware that I've been tripped up by types of foreign key references changing in the past in my own code and it's within the scope of the check framework to automate looking for these kinds of errors and warning users about them.
We can also nerf it again at some point in the futurewhen we feel we've had enough of a deprecation period. I just wish there was a way I could also check to see if any python code references these fields.
Merge in changes from kcrebound
Is there any plans to merge this PR? I use UUID as PKs on my models, and I can't see any workaround to use this project. |
@josh-stableprice the core of your PR is actually very valuable. The project recently got a massive update. Would you mind rebasing or cherry-picking the core of your changes ? Also adding some failing tests for for example UUIDs as PK would be desirable. Thanks in advance! |
Hi guys. Any reason to not merge this PR? The project is still unusable to many people just because of this :/ |
Can this PR be fixed and merged so the UUID issue can be solved please? Thanks! |
Will be solved by #176 |
Here is a basic implementation of the migration from integer primary key to allow UUIDS, with a check to warn people who use the field that they may need to migrate.
The check can be acknowledged and ignored by adding 'django_fsm_log.W001' to SILENCED_SYSTEM_CHECKS as documented here https://docs.djangoproject.com/en/2.2/ref/settings/#std:setting-SILENCED_SYSTEM_CHECKS
I also suggest adding some kind of warning on the main page and bumping the version number significantly to stop it being pulled in automatically by Poetry and Pipenv.
Fixes #34