[Fixes #91, #96] Rewrite renderer #106
Conversation
bluepichu
changed the title
| return fs.readFile(path.join(__dirname, `../posts/${post.guid}.md`)) | ||
| .then((content) => { | ||
| console.log(`Updating ${post.title.text}.`); | ||
| return db.posts.update({ guid: post.guid }, { $set: { content: content.toString() } }); |
There was a problem hiding this comment.
So. Let's talk about organisation. When I wrote this transfer code the first time, I too stuffed content into the same collection as the post metadata. But that seemed problematic to me. I realised that while posts and post data are inexorably linked, there are many cases in which you would want only the post data and not the post itself, but by linking them this way, you always force a fetch of the data. Now, I know that with a mongo server running on the same backend as the node server, making big queries is less of an issue. At the same time, it adds a lot of overhead to every query which we don't necessarily need
What I ended up doing was creating a separate collection for the posts, and linking them with ObjectIds. This means you have negligible lookup times for fetching the post from the metadata, and you also don't have to always grab the whole big content field. This might be an unnecessary optimisation, so I'm open to suggestion/debate.
There was a problem hiding this comment.
If you don't want post content, can't you just use the appropriate projection to not include it? i.e., db.posts.find({query}, {content: false})
There was a problem hiding this comment.
._. I forgot that operated on the cursor and not the object.
rip me
|
Also, if switching to |
| (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), | ||
| m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) | ||
| })(window,document,'script','//www.google-analytics.com/analytics.js','ga'); | ||
| <script> |
There was a problem hiding this comment.
Same JS beautifier issue. Fixing.
| } | ||
| db.authors.findOne({"id": user.id}) | ||
| .then((data) => { | ||
| console.log("here", data); |
There was a problem hiding this comment.
MAYBE I WANT TO KNOW WHEN IT GETS THERE
| @@ -287,7 +287,7 @@ content-row { | |||
| line-height: 1.4; | |||
|
|
|||
| &:last-child { | |||
| // margin-bottom: 0px; | |||
| margin-bottom: 0px; | |||
There was a problem hiding this comment.
Specifying 0px is redundant, 0 works
|
Regarding koa: I see no reason to switch at this time. |
|
nor do I |
| console.log(`Updating ${post.title.text}.`); | ||
| return db.posts.update({ guid: post.guid }, { $set: { content: content.toString() } }); | ||
| }) | ||
| })) |
There was a problem hiding this comment.
change this to })).then(() => process.exit(0)) so that the program terminates properly
|
Author's page should indicate that you are viewing an author's page |
|
same with tags |
|
I was hoping you wouldn't notice :| I'm planning a full overhaul of the UI there anyway, so I was planning to handle that post-this-PR. I could put in something temporary if you'd like. |
|
lol, is that the same for contact not including any contact info? |
|
...nope, that wasn't intentional, that was just me doing a poor job porting the previous template |
|
It occurred to me that not showing the filter "because modifying the templates is too hard" kind of undermines the primary goal of the changes of this PR, so I took the two minutes to add an ugly plaintext indicator (to be styled later). |
|
Can't tell, I defer to @zwade On Thu, Jul 7, 2016, 10:11 PM Matthew Savage [email protected]
|
|
Sorry, had to do a bunch of stuff this evening. Giving it the final rundown now |
|
yeah lgtm. Would recommend waiting to push to live until after we get a few more field tests in, but merging now |
This PR includes a total overhaul of the renderer and moves post content to the DB.
Please be very careful with this. Errors here would be bad. I'd like multiple LGTMs before merging this since it's so likely to break all the things.
@el1t: Feel free to throw all the style comments you want at me now. I cleared the comments on the original commit because they clutter the PR and aren't relevant anymore.