Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address review comments #214

Merged
merged 10 commits into from
Nov 3, 2023
Merged

Address review comments #214

merged 10 commits into from
Nov 3, 2023

Conversation

xluciano
Copy link
Contributor

@xluciano xluciano commented Nov 2, 2023

@mbanth mbanth assigned xluciano and unassigned mbanth Nov 2, 2023
@BrennanGit
Copy link
Contributor

Sorry I was in a meeting for a while. For reference, my exclude patterns now looks like this:

# The following patterns are to be excluded from the documentation build
tools
projects
test
xmos_cmake_toolchain
modules/**/thirdparty
modules/FreeRTOS
modules/sw_services
build
build_*
**CHANGELOG*
**LICENSE*
**README* 

The only other thing that leaps out is this error: index.rst:19: WARNING: Failed to create a cross reference. A title or caption not found: 'fwk_rtos_copyright'

I think this is because the section it refers to is included twice, once by each document. Might I suggest we just remove the licensing section from the top-level index.rst? I'd think it's visible enough as it is.

@xluciano
Copy link
Contributor Author

xluciano commented Nov 2, 2023

Sorry I was in a meeting for a while. For reference, my exclude patterns now looks like this:

# The following patterns are to be excluded from the documentation build
tools
projects
test
xmos_cmake_toolchain
modules/**/thirdparty
modules/FreeRTOS
modules/sw_services
build
build_*
**CHANGELOG*
**LICENSE*
**README* 

The only other thing that leaps out is this error: index.rst:19: WARNING: Failed to create a cross reference. A title or caption not found: 'fwk_rtos_copyright'

I think this is because the section it refers to is included twice, once by each document. Might I suggest we just remove the licensing section from the top-level index.rst? I'd think it's visible enough as it is.

I removed that reference and rephrased the text. See 4e07363.

@BrennanGit BrennanGit self-requested a review November 2, 2023 17:24
@BrennanGit
Copy link
Contributor

The last issue is this: programming_guide/platform.rst:3: WARNING: Duplicate explicit target name: "xtc tools"

Buit this appears to be fixed on develop already

@xluciano xluciano merged commit 5fad254 into xmos:develop Nov 3, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants