Move readme_rendered_at off of the versions table
#1238
Conversation
This column is never used in code other than for filtering/recording purposes. We do not put it on the `Version` struct, and don't need to be sending it over the wire every time we query that table.
|
Uuuhhhh but you did the opposite to this? 9c6412f#diff-2d162e07be0cf65cc1054b2128c60258 |
|
Am I not allowed to change my mind? :( The main thing I think I was opposed to was the amount of code that existed for no apparent reason, more than the schema itself. Also the schema makes much more sense to me when it's clear that exactly one row exists, and that the timestamp is always present in that case The main motivation here is to be able to remove the last manual |
|
I just want to be sure you're not going to be changing your mind back and forth forever :( |
|
Well I can at least say that moving this back again will require bringing back the manual |
src/version/mod.rs
Outdated
|
|
||
| diesel::update(self) | ||
| .set(readme_rendered_at.eq(now.nullable())) | ||
| diesel::insert_into(readme_renderings) |
There was a problem hiding this comment.
Doesn't this need to be an upsert? Or does insert_into handle upserts automatically? Occasionally, when we change the readme rendering, I rerender all the readmes, which should update this timestamp.
|
I added a test to encode the historical/contextual knowledge I had, it is indeed failing on this branch :) |
From Carol: > Occasionally, when we change the readme rendering, I rerender all the readmes, which should update this timestamp.
|
bors: r+ (I'm assuming you're good with this after I fixed that issue) |
Build succeeded |
This column is never used in code other than for filtering/recording
purposes. We do not put it on the
Versionstruct, and don't need to besending it over the wire every time we query that table.