Conversation
Instead of everything directly instead of wpPostsAggregate repeating with each post, instead it repeats the content inside of a wpPostsItem blank. This will allow for a wpPostsMeta that doesn't repeat.
This allows us to re-export commonly used imported types, like ResponseHeaders.
emhoracek
force-pushed
the
add-aggregates-meta-tags
branch
from
d95f8db to
2105d5b
Compare
spec/Common.hs
Outdated
|
|
||
| fauxRequester :: Maybe (MVar [Text]) -> Text -> [(Text, Text)] -> IO (Either StatusCode Text) | ||
| toWPResp :: (Text, H.ResponseHeaders) -> IO (Either StatusCode WPResponse) | ||
| toWPResp (body, headers) = return $ Right $ WPResponse headers body |
There was a problem hiding this comment.
it's confusing to see (body, headers) followed by headers body on the right hand side. I think standardizing on an order across the codebase is valuable. I would suggest headers and then body since that's the order the request has them from top to bottom.
There was a problem hiding this comment.
Yeah, originally it was just easier to paste , mempty) to the end of the line, but then I just made the toWPResp helper anyway. I should just make the helper add empty headers since they're empty for most of the tests.
spec/Main.hs
Outdated
| main = runTests | ||
|
|
||
| deadTests :: Spec | ||
| deadTests = do |
There was a problem hiding this comment.
this name is because...?
There was a problem hiding this comment.
There's a liveTests that is tests of a live server. I'll change the name XD
| else | ||
| do o <- wpRequest wpKey | ||
| case o of | ||
| do resp <- wpRequest wpKey |
There was a problem hiding this comment.
I'm imagining putting this in production and it retrieving a key from an older cached WP request that has no headers. Where will it fail to decode into a WPResponse and what will happen? We should either have a plan to explicitly clear the cache when we deploy this or be backwards compatible w/r/t the payloads already cached in Redis.
There was a problem hiding this comment.
Ooh, good thinking. I can add a fallback in the ToJSON implementation to avoid that.
There was a problem hiding this comment.
I meant FromJSON above. Added a fallback. It turns out you can write things like v .:? "some_field" .!= "some default text" which means, try to find some field, and if you can't find it, use this default.
This way users can deploy without blowing away the cache.
add wpNoMorePages
This was causing errors on CircleCI.
|
@dhartunian I used your suggestions -- take another look? |
2 participants
No description provided.