Dmitri, I committed callRecording.patch and recordButton.png in r7584.
Thank you for your good work on the useful feature that call recording
Here's an incomplete list of the issues I've found thus far:
- We require license headers in all files.
- We require you to @author classes not only when you've created them in
their entirety but also when you add methods, fields.
- Our Eclipse IDE formatter is not 100% in accord with our coding
convention. The best rule of thumb is to use it but to follow the rules
of the existing code which you modify.
- Eclipse IDE puts whitespace in empty lines but we don't want it and
it's easy to discover in the Compare Editor of Eclipse IDE (e.g. the
Team Synchronization perspective shows the changed files and launches
the Compare Editor on double click).
I tried to take care of the above but I found them to be a productivity
killer during my review.
- I don't find the Recorder interface and the API related to it in
OperationSetBasicTelephony and MediaAwareCall to be very good.
#startRecording takes a file name so it's not very clear to me what
happens when one decides to call it two times with different file names
without calling #stopRecording. Then #stopRecording doesn't really
identify which #startRecording it corresponds to. It seems to me that
you could've returned a Recorder from
OperationSetBasicTelephony#startRecording(fileName) and then you
could've had Recorder#stop instead of
OperationSetBasicTelephony#stopRecording. Anyway, I'd rather have the
comments of the other community members on it prior to asking you to
modify your code.
- I don't understand the "Save call to..." dialog depicted in the
screenshot save-call-to.png. When I browse for Location, it ask me not
only for a directory but it also requires a filename. Why not directly
open the Save file dialog upon clicking the Record button and have the
supported file formats in its filter?
- The notification displayed in the screenshot
call-saved-successfully.png should use our notification service, it
shouldn't be a dialog. Our notification service displays notifications,
for example, when somebody starts typing an instant message to you in a
new conversation or you receive an instant message.
- You're still using a MediaDeviceSession for the call recording while
you only need a Processor.