Code review comment for lp:~vds/usso/handle_order_of_parameters

Revision history for this message
Graham Binns (gmb) wrote :

Hi Vincenzo,

Thank you for this branch! There are a couple of things you need to add before you land this, but there's nothing that needs me to re-review it after you've made the changes, so feel free to make them and then mark the MP as approved.

[1]

149 +func (PLAINTEXT) Name() string { return "PLAINTEXT" }
150 +func (PLAINTEXT) Signature(

160 +func (HMACSHA1) Name() string { return "HMAC-SHA1" }
161 +func (HMACSHA1) Signature(

These all need some documentation comments.

[2]

275 +// Test the request signing with oauth_signature_method = PLAINTEXT
301 // Test the request signing with oauth_signature_method = SHA1

We should avoid the "Test the..." style when writing documentation for tests. Instead, we should write things like:

  // It is possible to sign a request with a PLAINTEXT oauth_signature_method.
  // It is possible to sign a request with a SHA1 oauth_signature_method.

Put another way, the documentation on a test should be something that can be declared true if the test passes, false otherwise.

review: Approve (code)

« Back to merge proposal