On Thu, Jan 13, 2022 at 2:38 PM Bossart, Nathan <bossartn@amazon.com> wrote:
> Here is another rebase for cfbot.
I've committed 0001 now. I don't see anything particularly wrong with
the rest of this either, but here are a few comments:
- I wonder whether it might be better to promote the basic archiving
module to contrib (as compared with src/test/modules) and try to
harden it to the extent that such hardening is required. I think a lot
of people would get good use out of that. It might not be a completely
baked solution, but a solution doesn't have to be completely baked to
be a massive improvement over the stupidity endorsed by our current
documentation.
- I wonder whether it's a good idea to silently succeed if the file
exists and has the same contents as the file we're trying to archive.
ISTR that being necessary behavior for robustness, because what if we
archive the file and then die before recording the fact that we
archived it?
- If we load a new archive library, should we give the old one a
callback so it can shut down? And should the archiver considering
exiting since we can't unload? It isn't necessary but it might be
nicer.
- I believe we decided some time back to invoke function pointers
(*like)(this) rather than like(this) for clarity. It was judged that
something->like(this) was fine because that can only be a function
pointer, so no need to write (*(something->like))(this), but
like(this) could make you think that "like" is a plain function rather
than a function pointer.
--
Robert Haas
EDB: http://www.enterprisedb.com