I stumbled about the "uninitialized changeset" problem while extending Review Board for web-based post reviews. In my opinion the code in not clean even if it is not relevant in official releases at the moment. Taking your comment into account my proposal seems to be a workaround only. If changenum + changeset support belong together logically (as indicated by you) I recommend to no longer ignore the NotImplementError exception. 

My new proposal is to remove the following lines
            except NotImplementedError:
                # This scmtool doesn't have changesets
                pass 

Did I overlook something?

I'd like to hear more about what you're doing. Having a web UI for post-commit reviews is one of our most requested features, so it would be great if we could get a general solution.

Having the changenum field tied to changesets is an artifact of the way that we've done things so far, but it's not necessarily how things need to be. If there's a good reason to change it (and post-commit reviews would be a good reason), we definitely can.

I think it's fine to remove "pass" and to properly validate whether changesets are supported (when passing changenum) is ideal. So maybe replace "pass" with raising some new error result saying that changesets aren't supported.

This needs to then be hooked up in reviews/views.py and webapi/resources.py to provide the errors in a meaningful way.

The code dealing with all this is really old. As in, one of the very first things ever written, back when forms, our error processing, our API, and really half of Django either didn't exist or didn't work the same way. So it shouldn't be a surprise that it's not the cleanest and needs some love.

@David:
In 2009 and 2010 I implemented post-commit reviews for both Perforce and TFS at Nero. Unfortunately I had not the time release the code to the public due to various impediments (your switch from subversion to git / job change). Currently my company is working with Subversion and I started to publish my modifications right from the beginning. Please have a look at http://philipphenkel.github.com/post-reviewboard

I'm on holiday next week and would to start a mailing list discussion about post commits afterwards. It would be great if we find a way to smoothly integrate post commit reviews into the official master. I'm sure your have a lot of requirements and ideas about this topic.

@Chris:
Maybe we can re-use the ChangeSetError exception? 

            try:
                changeset = repository.get_scmtool().get_changeset(changenum)
            except NotImplementedError:
                # This scmtool doesn't have changesets
                self.errors['changenum'] = forms.util.ErrorList(['Changesets are not supported.'])
                raise ChangeSetError()

It gives a clear error message and should not need any additional code changes. How do you think?

That sounds great. Can you post that in a new diff here?