-
Notifications
You must be signed in to change notification settings - Fork 6
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
Store pid of the backend when connecting to Postgres #71
Conversation
a7a136d
to
ffceebf
Compare
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.
LGTM. Only suggestion (apart from style nitpicking) would be to use a newtype for the PID, but it might be less convenient I guess.
getBackendPidIO :: Connection -> IO Int | ||
getBackendPidIO conn = do | ||
withConnectionData conn "getBackendPidIO" $ \cd -> do | ||
pure (cd, cdBackendPid cd) |
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.
I would dispense with the do
here, to be honest, but I guess from our hlint.yaml there is no in-house policy against redundant do
?
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.
While do
is technically redundant, it's there to appease haskell-mode formatter in emacs to do the right thing 👻
@@ -370,8 +396,7 @@ runQueryImpl fname conn sql execSql = do | |||
, statsValues = statsValues cdStats + (rows * columns) | |||
, statsParams = statsParams cdStats + paramCount | |||
} | |||
-- Force evaluation of modified stats to squash a space leak. | |||
stats' `seq` return (cd {cdStats = stats'}, (either id id affected, res)) | |||
return (cd {cdStats = stats'}, (either id id affected, res)) |
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.
What about using pure
rather than return
since it's what you used everywhere else ?
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.
Right, although there's tons of return in other places, so whether I replace this one or not is irrelevant. Let's leave it for later when hlint is enabled for this repo, it can be then done systematically for all occurrences.
I added newtype for the pid. |
No description provided.