Code review comment for lp:~morita-kazutaka/nova/snapshot-volume

Revision history for this message
Brian Waldon (bcwaldon) wrote :

> > Looking good! I'm more than happy to send this in once a couple minor things
> > are fixed:
> >
> > 108: This description doesn't seem right. Is this a copy/paste error?
>
> Fixed, thanks!
>
> >
> > Should the "snapshot_name_template" be used in places like line 23, 58, 93,
> > 388 and 419? This is more of a question, as I do not actually know the
> answer
> > :)
>
> The name "snap-xxxxxxxx" is the notation of EC2 snapshot id, so I think we
> shouldn't use "snapshot_name_template" for those.
>
> I've also fixed pep8 errors.

Sounds good to me.

review: Approve

« Back to merge proposal