[LEDE-DEV] [PATCH v2 2/3] base-files: put board_name into separate file

Mathias Kresin dev at kresin.me
Wed May 17 15:05:19 PDT 2017


17.05.2017 21:08, Roman Yeryomin:
> On 17 May 2017 at 21:39, Roman Yeryomin <leroi.lists at gmail.com> wrote:
>> On 17 May 2017 at 16:19, Mathias Kresin <dev at kresin.me> wrote:
>>> Hey Roman.,
>>>
>>> independent of the work done by you, I worked on exactly the same
>>> topic. But I've pushed it a bit further by switching all targets to
>>> the generic boardname function and a few targets to the generic board
>>> detection in basefiles/preinit.
>>>
>>> Furthermore I've converted some targets to get the boardname from the
>>> device tree compatible string as suggested by John. Till now I wasn't
>>> comfortable to push it somewhere, but with my latest changes it should
>>> work in theory. You can find the changes at
>>> https://git.lede-project.org/?p=lede/mkresin/staging.git;a=shortlog;h=refs/heads/boarddetection.
>>> What is missing: testing!
>
> Some of your changes would be ok to me if:
> - board.sh would be used instead of functions.sh (which I would like
> to split into several files anyway) for $(board_name)

I guess we all got your point. But I'm sorry to say, that isn't subject 
of my changes.

> - related changes would be squashed into one commit, otherwise it's
> hard to perceive/track the changes, IMO more commits is not always
> better

The commits will be most likely squashed before I commit them. But at 
least during review I prefer smaller changes instead of a single patch 
with >100 lines.

>
> also this one is a fail:
> https://git.lede-project.org/?p=lede/mkresin/staging.git;a=commitdiff;h=af3ef20591e6e9e4ee2cf1b1fc58012df68445ba

Hopefully someone beats me with a stick if I every reply during a review 
with "fail" and without any explanation what is wrong. This isn't 
helpful in any kind and just a waste of your and my time.

But I can give you a quick summary why the commit is required:

No target_board_name() => target_board_detect() isn't called => 
/tmp/sysinfo/board_name isn't populated => board_name() has nothing to 
return.

And as the commit message states, it is also meant to unify the way the 
board detection is done between targets (to not fool more people like I 
was fooled because of these target specifics).

Mathias



More information about the Lede-dev mailing list