Given non-null default value to the database#31
Conversation
|
This should be fine indeed. To verify we should set https://github.com/ChristophWurst/twofactor_admin/blob/66c91e16dbcc6fb046b966264d7acf964821b6d2/.travis.yml#L21 to 10.3. If CI passes the patch fixes #20 |
|
Should I just do this or wait for inspection? I tried it in a separate branch to check the travis tests' results. With |
Yes, please change this here.
Good catch. I'm fine with having a separate PR for that, but in order to mark #20 as fixed I want to have working CI checks that verify it. And as you tested, Travis won't pass on mysql/mariadb once a newer version is used. |
7de1537 to
9e1ae43
Compare
|
OK, I changed the maradb version 10.3 as intended. As already stated the CI failed. To get it running again, I added a comparably dirty hack to restore the autoincrement flag in the database. With that the CI succeeded and @ChristophWurst might want to have a look at it. |
0dbcad6 to
2857b05
Compare
ChristophWurst
left a comment
There was a problem hiding this comment.
Squashed & rebased. Changes look good and CI is happy. Let's get this in 👍
Set a default (non-null) value during migration allows to use broader set of mariadb versions.
This PR is motivated by the discussion on #20.
Fixes #20
At least on my machine this allows the app to be installed successfully. I think this should not affect any future versions as it simply set a feasible (in terms of the DDL) default value, which seems reasonable.
It has to be verified by the main developer that the default value of
0is a valid solution: That is: Is theexpirescolumn really a unix timestamp or some other format? My understanding is yes (which is safe) but I am not 100% sure from reading through the code.