On Thu, Jan 13, 2022 at 2:38 PM Bossart, Nathan <bossa...@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