Code review comment for lp:~gagern/bzr/bug560030-include-bash-completion-plugin

Revision history for this message
Martin von Gagern (gagern) wrote :

Thanks for the review, Vincent! Probably won't have time to deal with it before 27 April or so, so I'm putting this on hold until then.

Testing is a real problem here: the things I'd be most concerned about or interested in is bash syntax and behaviour, and obviously testing these involves executing bash, which can't be done from within python only. Maybe I can write tests that check whether bash is available, and skip if it isn't.

I think this approach is the only thing that could really catch regressions, in particular the one I recently introduced and fixed on trunk. This is the only thing that can actually check the behaviour of the hardcoded template around the simple dynamically generated code.

Another thing I might do is refactor stuff so that data aquisition and code generation are separated. That way, we could ensure that the data aquisition does the correct thing, and hopefully keep the code generation simple enough so it will be easy to check. This might also open the door for other shells, zsh in particular, so it might be a good thing even without tests. But it's a major step, so it will take a bit of time.

« Back to merge proposal