[ccan] base64 implementation - feedback requested

David Gibson david at gibson.dropbear.id.au
Wed Jul 30 23:16:30 EST 2014


On Fri, Jul 11, 2014 at 09:05:12PM +1000, Peter Barker wrote:
> 
> I would appreciate some feedback on a base64 implementation for ccan.
> 
> The first branch is an adjustment to ccan/tap; is_str and is_mem
> implementations giving perl-style, 'expected "foo", got "bar"'
> errors.

So, some meta-feedback first.

> The following changes since commit 8b0bdb090e2882aa431e89f4bc7aa4736e9e2838:
> 
>   structeq: new module. (2014-06-25 15:53:34 +0930)
> 
> are available in the git repository at:
> 
>   https://github.com/peterbarker/ccan.git tap-is_foo-defines

Pull URLs are nice to have for merge time, but for ease of review, the
preferred format is to send as patches - one patch per mail in a
thread.  git send-email will do this for you from your own git tree.

You also need some work on your commit messages, the usual format for
ccan is:

~~~
modulename: Short description

Longer description.   This should address why the change is desirable,
e.g. "adds feature foo, useful for bar", or for a bug fix, an
explanation of why the original code was incorrect (for subtle
one-line fixes this may need to be longer than the patch itself).

Signed-off-by: <you at example.com>
~~~

> 
> for you to fetch changes up to 6a4e2f3de7e6fb8b0438add710477a93a0de355f:
> 
>   remove is_num as too ambitious got the time being (format string problems
> in output); add description for is_mem (2014-07-11 19:59:52 +1000)
> 
> ----------------------------------------------------------------
> Peter Barker (2):
>       add is_num, is_mem and is_str
>       remove is_num as too ambitious got the time being (format string
> problems in output); add description for is_mem

Things you send for review and/or merge should generally have
sanitised history - fold fix patches into the original and generally
make the commit series look like logical steps, rather than the series
of trials and errors it actually was.  git rebase -i is the tool for
this job.

> If this is entirely the wrong way about getting this code into ccan - please
> let me know the *right* way to do things :-)

Ayup.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/ccan/attachments/20140730/4a4177fb/attachment-0001.sig>


More information about the ccan mailing list