On 3/11/20 2:59 PM, Nick Rosbrook wrote:
> Looks good, I just have two small comments:
>
>> diff --git a/tools/golang/xenlight/xenlight.go
>> b/tools/golang/xenlight/xenlight.go
>> index 56fa31fd7b..808b4a327c 100644
>> --- a/tools/golang/xenlight/xenlight.go
>> +++ b/tools/golang/xenlight/xenlight.go
>> @@ -1111,3 +1111,24 @@ func (Ctx *Context) PrimaryConsoleGetTty(domid
>> uint32) (path string, err error)
>> path = C.GoString(cpath)
>> return
>> }
>> +
>> +// int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config
>> *d_config,
>> +// uint32_t *domid,
>> +// const libxl_asyncop_how *ao_how,
>> +// const libxl_asyncprogress_how
>> *aop_console_how)
>
> Conventionally, we want to have comments for exported functions along
> the lines of:
>
> // DomainCreateNew creates a new domain with config, and returns
> its Domid on success.
> // A non-nil error is returned if config cannot be marshaled, or
> an error occurs within libxl.
>
> Besides being easier to read, it makes documentation more clear on
> godoc/pkg.go.dev.
Yes, absolutely, that's something we need to change before we declare
"1.0". But that should probably be done in a series which changes all
such comments together.
>> +func (Ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
>
> Capitalizing "Ctx" here is a little weird to me. Since it's only the
> receiver name, there's no effect, but since capitalized identifiers
> have special-meaning in other contexts, I would avoid doing this.
I c&p'd this from another method and just changed the signature. It
probably would be good to make all of them lower-case. I may change
this one on check in though.
> I only point those out in case you want to change it on check-in. Besides
> that,
>
> Reviewed-by: Nick Rosbrook <rosbro...@ainfosec.com>
Thanks!
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
