On Wed, Feb 05, 2014 at 11:51:29AM +0100, Santi Raffa wrote:
> On Wed, Feb 5, 2014 at 11:09 AM, Jose A. Lopes <[email protected]> wrote:
> > For example
> >   Algo: add GetRepeatedKeys
> > instead of
> >   algo: ...
> >
> > Also,
> >   OpCodes: modify ...
> > instead of
> >   opcodes: Modify ...
> 
> I can see why you'd want OpCodes instead of opcodes, but Algo is not
> consistent with the filename algo.py.

I see! I didn't know these were referring to files.  I understood them
as tags, for example, the opcodes tag would mean that you did some
work around opcodes, maybe in the opcodes file or others.

There's no need to put the affected files in the title because they
are part of the patch.  The important part here is to make sure that
someone who is unfamiliar with your patch series can look at the title
and understand what you are trying to do.

The refactoring you did seems pretty good.
Only two things:

Instead of
  algo: add GetRepeatedKeys
how about?
  Add helper function to detect duplicate keys in OS params

This one I don't understand
  error: constants: add visibility levels
Is it the following?
  Add visibility levels to errors

Thanks,
Jose

> I've gone through all commits to rework the commit messages:
> 
> commit c0dfb2c65b252b7ca1a8887cddc513f7cd73a7cf
> Author: Santi Raffa <[email protected]>
> Date:   Tue Feb 4 14:11:52 2014 +0000
> 
>     NEWS: update with public and private paramters
> 
>     Also warn about debug mode.
> 
> commit 383768683be3d6c42d84ddb3dbef93e6ec394445
> Author: Santi Raffa <[email protected]>
> Date:   Sun Feb 2 22:39:56 2014 +0000
> 
>     SimpleFillOS: add unit tests for OS parameter merging
> 
>     Adds tests to ensure OS parameters are dealt with consistently.
> 
> commit 558fcfdb336153c297ce59c9bde5ee17e89eb2d7
> Author: Santi Raffa <[email protected]>
> Date:   Wed Jan 29 19:08:07 2014 +0000
> 
>     luxid: give stern warnings about debug mode
> 
>     Luxid as it is can leak private and secret parameters by logging
>     all requests as they arrive, before any preprocessing is done.
> 
>     Warn the user stern warnings about this.
> 
> commit 6d88b090125607cd7c0fac5065102c3998e2f68b
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 14:38:18 2014 +0100
> 
>     OpCodes: modify InstanceReinstall for private, secret parameters
> 
>     Modify InstanceReinstall to accept and process private and secret
>     parameters
> 
> commit f476684f577f7b1dd1d62803e5dcde8b0ce6d2ab
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 14:37:20 2014 +0100
> 
>     OpCodes: modify InstanceCreate for private, secret parameters
> 
>     Modify InstanceCreate to accept process private and secret parameters
> 
> commit e2e50e1bd8bb069c50d0efb7b103aed5aeacd736
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 14:35:55 2014 +0100
> 
>     OpCodes: modify ClusterSetParams for private parameters
> 
>     Modify ClusterSetParams to accept and process private parameters.
> 
> commit f8df2dd6334da2e236b49c2cc810b7d14710f48a
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 13:52:20 2014 +0100
> 
>     OpCodes: Modify InstanceSetParams for private parameters
> 
>     Modify InstanceSetParams to accept and process private parameters.
> 
> commit 59adf60051ca229a85a31c0e991772c9e913a0ef
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 11:46:52 2014 +0100
> 
>     CLI: add parameters for private and secret OS parameters
> 
>     Define the CLI parameters for private and OS parameters.
> 
> commit 4747f8d26636d4381118ac95b22653e12e6b0542
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 14:07:27 2014 +0100
> 
>     Add private OS parameters to the cluster and instance configuration
> 
>     This updates objects, constructors and mocks for Instance and Cluster
>     objects in Python and Haskell.
> 
> commit 7b122f0942d21465666e4c03c5a0ff1f6ea76367
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 11:12:58 2014 +0100
> 
>     serializer: emit and encode Private values
> 
>     For inbound data the simplest, safest thing to do is to traverse all
>     JSON right after encoding and search for private parameters by key.
> 
>     This ensures that all consumers of this data get Private values
>     transparently and consistently; the serializing methods don't have to
>     worry about trying to understand what kind of JSON it is receiving.
>     It also minimizes the surface area of codebase that is exposed to the
>     values directly.
> 
>     The downside is it adds a ~5% time overhead to all JSON decoding as
>     measured through:
> 
>     $ time make hs-test-py_compat_types
> 
>     As far as encoding to JSON is concerned, the serializer will redact
>     Private values unless told to do so explicitly (e.g., for tests).
> 
> commit 60198244cd0f14f7ed153ffa5e26e7da228c80e3
> Author: Santi Raffa <[email protected]>
> Date:   Tue Jan 14 16:17:01 2014 +0100
> 
>     Add Private types to Python, Haskell
> 
>     This commit adds the private containers to Python and Haskell.
> 
> commit eb4528d5214f7a0f11da364731e82017e5706ea2
> Author: Santi Raffa <[email protected]>
> Date:   Tue Jan 14 13:56:01 2014 +0100
> 
>     error: constants: add visibility levels
> 
>     This commit is useless.
> 
> commit 78bcf50fee0ba15ffa1b81b0e642ccf477819596
> Author: Santi Raffa <[email protected]>
> Date:   Wed Feb 5 11:21:01 2014 +0100
> 
>     algo: add GetRepeatedKeys
> 
>     We do not want public, private and secret parameters to have
>     overlapping keys. This function implements this check.
> 
> commit 9be73718be2c041646e766e6589d9c50a3732a39
> Author: Santi Raffa <[email protected]>
> Date:   Fri Jan 24 11:28:17 2014 +0100
> 
>     OpCodes test: fix argument order (expected/but got)
> 
> commit 071db213ef5dad8b2ee6e7c1b0e521769bc664ca
> Author: Santi Raffa <[email protected]>
> Date:   Wed Jan 22 14:33:55 2014 +0100
> 
>     RPCs: add docstrings for instance_os_add
> 
>     This RPC's instance_osp input has a non-obvious, non documented
>     type. This patch adds documentation to this RPC.
> 
> 
> -- 
> Raffa Santi
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
> 
> 
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores

-- 
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to