[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] build: Rename as-insn-check to as-insn-add



>>> On 22.02.18 at 13:39, <george.dunlap@xxxxxxxxxx> wrote:
> On 02/22/2018 12:22 PM, Jan Beulich wrote:
>>>>> On 22.02.18 at 12:41, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 22/02/18 11:33, Jan Beulich wrote:
>>>>>>> On 22.02.18 at 11:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> as-insn-check mutates the passed-in flags.  Rename it to as-insn-add, in 
>>>>> line
>>>>> with cc-option-add.  Update all callers.
>>>> I'm not convinced - cc-option-add makes relatively clear that
>>>> something is being added to the options passed to CC. If I
>>>> take as-insn-add this way, the macro would need to add an
>>>> insn to the AS invocation. While I agree as-insn-check doesn't
>>>> make clear that it adds any options, I still find this less
>>>> misleading than the suggested new name. Let's see what
>>>> others think.
>>>
>>> I'm open to better name suggestions.
>> 
>> The best I can come up with is, well, as-insn-check, as that
>> reasonably describes at least part of what the construct does.
>> as-insn-check-and-add-option, besides being too long, isn't
>> meaningfully better.
> 
> We're definitely getting into bikeshed territory here.

Indeed, but I think a change in name should be an improvement,
not going from one questionable name to another questionable
one.

>  I agree with
> Andy that 'check' doesn't really convey that something changed.  Is the
> check-and-add "add it if it doesn't exist already"?  Or add it if some
> other check passes / fails?

It is "check if this piece of assembly assembles and add the
provided option to the indicated variable", extended by Roger's
patch to "..., and add the other provided option if it doesn't
assemble".

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.