On 11/14/19 5:20 PM, Cole Robinson wrote:
On 10/22/19 9:24 PM, Laine Stump wrote:
> +int
> +virDomainNetDefRuntimeValidate(const virDomainNetDef *net G_GNUC_UNUSED)
> +{
> + /* Unlike virDomainNetDefValidate(), which is a static function
> + * called internally to this file, virDomainActualNetDefValidate()
> + * is a public function that can be called from a hypervisor after
> + * it has completely setup the NetDef for use by a domain,
> + * including possibly allocating a port from the network driver
> + * (which could change the effective/"actual" type of the NetDef,
> + * thus changing what should/shouldn't be allowed by validation).
> + *
The comment here claims the function is named
virDomainActualNetDefValidate. I think that naming is more consistent
rather than invent a new 'runtime' naming scheme, event though runtime
is more semanticly descriptive. (though TBH I wouldn't mind
s/actual/runtime/ in the codebase anyways, the 'actual' naming always
confuses me)
Well, "actual" is describing *what* it is, and "runtime" is describing
*when* it is. In most cases I've been fine with the "actual" name, but
this time it didn't seem to adequately convey the usefulness of the
function, so I decided to try something else out, and missed the
function name in the comment.
Comment should be fixed either way, I'll leave it up to you whether to
actually rename the function.
Thinking about it more, I guess it's better to remain consistent
everywhere. Maybe someday someone will find some other adjective that
works better than actual in all circumstances (it might even be
"runtime", my personal mental jury is still out), and will switch all
occurrences at once.
Series:
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
I think there's a conflict in patch 7, git am gave me some confusing
results, so make sure the code you are moving is incorporating and
possible new changes that landed there since this was posted.
Yeah, I had rebased and was going to resend, but you reviewed the
original before I got around to it.