Update DTS file for Nexthops BMC card B27#591
Conversation
Signed-off-by: Chandrasekaran Swaminathan <chander@nexthop.ai>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
paulmenzel
left a comment
There was a problem hiding this comment.
I am sorry, this is hard to review. Please make it smaller commits despite it being squashed in the end. (No idea, why the SONiC admins don’t change that GitHub setting.)
Anyway, SONiC has several boards, so please don’t use generic summaries/title like Update DTS file.
Please also amend the commit message to include a description (or should you split as asked above) several descriptions.
The DTS file doesn't represent a generic hardware. Its a patch file to add a DTS file to the kernel build that represents our card. Doesn't affect any other vendor. Having an incremental patch to update this single DTS file seem overkill as no other vendor is impacted by it. |
Sorry, that is a misunderstanding. It’s not about, if other devices are impacted, it’s about being able to review stuff, and to understand changes in several years, once Nexthop moved on and stopped supporting code. |
A far earlier way to review a DTS file which is submitted as patch, imho, is to apply the patch and look into the DTS file directly. If there is a better way to add the DTS file to the SONiC Kernel let me know. |
Signed-off-by: Chandrasekaran Swaminathan <chander@nexthop.ai>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This is still not addressed. Please add the device name (NextHop B27 R0?) to the commit message summary/title, and the merge/pull request summary/title. |
Sorry about missing this. Done. |
Update the DTS file to remove unused sections and add sections that are relevant to our card.