thesofproject / sof-docs

Documentation for SOF
Other
18 stars 73 forks source link

Edit vendor-specific toc for better navigation; fix build breaks #434

Closed deb-intel closed 2 years ago

deb-intel commented 2 years ago

Signed-off-by: Deb deb.taylor@intel.com

mwasko commented 1 year ago

@deb-intel may I ask why are you merging your PR changes without any review or feedback? The vendor specific path was explicitly defined to have cavs content in separate folder with plan to put the ace folder next to it (incoming in next PRs). Please restore the original structure that was there before.

deb-intel commented 1 year ago

@mwasko I could not see many of the updates without first publishing it (the .pu and .dot images do not appear in any of my previews). I can always easily (and quickly!) make changes and republish.

Based on your explanation, I believe that my updates enhance your mission. The vendor-specific directory stands out as an option separate from SOF-XTOS and SOF-Zephyr. Intel cAVS Arch is the first item under Vendor-Specific; the ACE folder can be added to the Vendor-Spec dir. Note also that this reduces the nesting by one dir. If my understanding is incorrect, please let me know and I will change it back to what you had with my apologies.

Revised Structure Firmware --SOF-XTOS --SOF-Zephyr --Vendor-Specific (these are 3 separate dirs with cAVS and future ACE under this dir) ----Intel cAVS ----ACE

Your Structure Firmware --SOF-XTOS (these are 2 dirs with vendor-specific appearing as a sub-dir) --SOF-Zephyr ----Vendor-Specific --------Intel cAVS --------ACE

mwasko commented 1 year ago

@mwasko I could not see many of the updates without first publishing it (the .pu and .dot images do not appear in any of my previews). I can always easily (and quickly!) make changes and republish.

@deb-intel I am ok with the publishing first and then doing any required updates, but I would like to have a chance to review the modifications you are introducing before they get merged - just ping me as a reviewer to get a feedback. This way we could avoid this vendor specific file tree misunderstanding.

Revised Structure Firmware --SOF-XTOS --SOF-Zephyr --Vendor-Specific (these are 3 separate dirs with cAVS and future ACE under this dir) ----Intel cAVS ----ACE

This revised structure that you merged is adding Vendor-Specific section but does not provide Intel folder where we could store cAVS, ACE and common to Intel architectures chapters. It will also get a bit messy when other vendors will start to put their arch specific chapters in the same Vendor-Specific folder.

Your Structure Firmware --SOF-XTOS (these are 2 dirs with vendor-specific appearing as a sub-dir) --SOF-Zephyr ----Vendor-Specific --------Intel cAVS --------ACE

This is not what my structure look like. If you look at the code carefully you will see that the tree was: Firmware --SOF-XTOS --SOF-Zephyr --Intel (to avoid multiple sub-dirs each vendor add their folder name at the same level as same level as XTOS differentiation takes place) ----cAVS (then on sub dir we define folders for specific architectures of selected vendor) ----ACE (to be added next)

I want to highlight that I am talking about file structure tree in the repository. I am open for the discussion on how we present them in architectures/firmware/index.html. I have directly put the reference to Intel cAVS under Vendor Specific Title (that is what probably mislead you). The idea was to to avoid another level of clicking to Intel architecture first.

deb-intel commented 1 year ago

@mwasko Sounds good! I will look over this and make changes for you to see. Thanks for your patience!