nickva commented on code in PR #5666:
URL: https://github.com/apache/couchdb/pull/5666#discussion_r2357397998
##########
src/couch/src/couch_file.erl:
##########
@@ -621,7 +621,7 @@ handle_info({'DOWN', Ref, process, _Pid, _Info},
#file{db_monitor = Ref} = File)
false -> {noreply, File}
end.
-format_status(_Opt, [PDict, #file{} = File]) ->
+format_status([PDict, #file{} = File]) ->
Review Comment:
Oh no, sorry, I think there was a misunderstanding. I meant this particular
`format_status/2` in `couch_file` seems silly, not just because it formats the
process dict (this is the only one in our code base that does it), but it's
silly altogether, as it ends up formatting stuff that's already in the state
anyway, so we can remove it.
Notice how the `Fd` and `filepath` are in the `#file{}` record which is the
gen_server state. So a default "status" of that will include both the Fd and
the filepath (plus an offset and a reference). In other words, we can remove
format_status in `couch_file` and not lose very much with it.
But in other modules these format_status callbacks can be useful for cases
when the state may be huge, or may have sensitive info like passwords or
cookies or something, so we should keep those and change them to handle maps.
Maybe some can be tweaked or removed but we can go through them individually
probably.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
