On 2018-08-25 15:33, Max Reitz wrote: [...]
> Having a central Error object doesn't really make sense. Whenever the > block job wants to return an error, it should probably do so as "return" > values of methods (like .run()). Same for ret, of course. > > I understand that this is probably really only possible after v2 when > you've made more use of abort/commit. But still, I don't think this > patch improves anything, so I would leave this clean-up until later when > you can really do something. > > I suppose the idea here is that you want to drop the errp parameter from > job_completed(), because it is not going to be called by .exit(). But > the obvious way around this would be to pass an errp to .exit() and then > pass the result on to job_completed(). You know what, I wrote a really long reply and in the end I realized you basically did exactly what I wanted. (Or, rather, I gave two alternatives, and you did one of them.) I noticed that having a central error object may still make sense; .create() shows why, jobs usually fail in .run() and then you need to keep the error around for a bit. You can either pass it around, or you just keep it in Job (those are the two alternatives I gave). (So I said, either rip out Job.error/Job.err and Job.ret completely, or keep the iff relationship between Job.err and Job.ret, so together they give you a meaningful state.) Now by setting Job.ret and Job.err basically at the same time (both are results of .run(), and whenever Job.ret is set somewhere else, it is immediately followed by job_update_rc()), the iff relationship between Job.err and Job.ret persists. So that's good! (In that case I don't know why you removed the "iff" part from the comment, though.) The only remaining question I have is why you still pass job->ret to job_completed(). That seems superfluous to me. Max (No, I don't know what's up with me and completely misunderstanding this series.)
signature.asc
Description: OpenPGP digital signature
