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

doc: posix: structural reorganization of posix docs #64540

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Oct 30, 2023

Revise the structure of the POSIX API docs. This separates related items out to dedicated pages, see preview.

Screenshot 2023-10-30 at 7 58 35 PM

@cfriedt cfriedt requested review from kartben and ycsin October 30, 2023 06:11
@cfriedt cfriedt force-pushed the posix-relnotes-extra-3.5.0 branch from ba01bec to 38cce65 Compare October 30, 2023 20:49
@cfriedt
Copy link
Member Author

cfriedt commented Oct 30, 2023

Was trying to disrupt all of the red (✅ = complete, 🏁 = nearly done, ⚠️ = in progress, ❌ = not yet started).

Suggestions @kartben ?

@cfriedt cfriedt force-pushed the posix-relnotes-extra-3.5.0 branch 3 times, most recently from ebc48b6 to 5db507a Compare October 30, 2023 21:24
@cfriedt cfriedt requested review from keith-packard, nashif, aescolar and a team October 30, 2023 23:55
@cfriedt cfriedt marked this pull request as ready for review October 30, 2023 23:59
@zephyrbot zephyrbot added area: Portability Standard compliant code, toolchain abstraction area: CMSIS API Layer CMSIS-RTOS API Layer labels Oct 30, 2023
@cfriedt
Copy link
Member Author

cfriedt commented Oct 31, 2023

@kartben the things I would really like to do (in the future) are

  • modify the Options in .rst so that POSIX functions link to a ref in doxygen (forward-ref)
  • add a minimal doxygen entry for each POSIX function
  • include a link in that doxygen entry back to the POSIX Option that it belongs to in .rst (backward-ref)

The problem is that there are a lot of POSIX functions.

Between the lines, that means it will be difficult to maintain manually (will bitrot, become inaccurate, etc).

IMHO, it makes a lot more sense to script this.

This PR intentionally doesn't include most of the above, in case we would like to backport it to v3.5-branch.

@cfriedt cfriedt force-pushed the posix-relnotes-extra-3.5.0 branch from 5db507a to bd6580b Compare November 8, 2023 17:40
@cfriedt cfriedt requested review from aescolar and ycsin November 8, 2023 17:44
@cfriedt cfriedt force-pushed the posix-relnotes-extra-3.5.0 branch 2 times, most recently from 0cdd9bc to 7f83312 Compare November 8, 2023 17:49
ycsin
ycsin previously approved these changes Nov 9, 2023
fabiobaltieri
fabiobaltieri previously approved these changes Nov 9, 2023
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this effort! Quite a few comments, but mostly cosmetics/formatting.

doc/services/portability/posix/overview/index.rst Outdated Show resolved Hide resolved
doc/services/portability/posix/overview/index.rst Outdated Show resolved Hide resolved
doc/services/portability/posix/overview/index.rst Outdated Show resolved Hide resolved

.. _posix_undefined_behaviour:

.. note::
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you make it a subheading instead of a note, and have the heading spelled "Undefined behavior" (it will arguably make it stand even more, and will make it more clear for folks why they landed there when clicking on one of the "unsupported" links), then you can :ref:`posix_undefined_behaviour` directly in your table and it will already have a meaningful and usable title

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not crazy about that standing out, to be honest.

:header: Symbol, Support, Remarks
:widths: 50, 10, 50

_POSIX_JOB_CONTROL, -1, :ref:`†<posix_undefined_behaviour>`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The obelus feels very lonely and tiny in this column (and also hard to click for people and not very accessible).
See my comment re: how to change the note to a heading, and then this can simply become

Suggested change
_POSIX_JOB_CONTROL, -1, :ref:`†<posix_undefined_behaviour>`
_POSIX_JOB_CONTROL, -1, :ref:`posix_undefined_behaviour`

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, again, I'm not super happy about lumping so many things into undefined behaviour. Ideally the gap would be significantly smaller but that also takes time and work. Additionally, I'd like to look at a better solution for generating these tables rather than maintaining them by hand.

doc/services/portability/posix/overview/index.rst Outdated Show resolved Hide resolved
doc/services/portability/posix/overview/index.rst Outdated Show resolved Hide resolved
doc/services/portability/posix/conformance/index.rst Outdated Show resolved Hide resolved
ycsin
ycsin previously approved these changes Nov 14, 2023
@cfriedt cfriedt force-pushed the posix-relnotes-extra-3.5.0 branch 3 times, most recently from 5065dbb to 83e74f9 Compare November 20, 2023 20:47
@cfriedt
Copy link
Member Author

cfriedt commented Nov 20, 2023

@kartben - does it look OK for now? These POSIX doc changes were meant to be a backportable precursor to additional features in POSIX and POSIX configuration (which probably cannot be backported).

@cfriedt cfriedt force-pushed the posix-relnotes-extra-3.5.0 branch from 83e74f9 to eab576d Compare November 20, 2023 20:52
@kartben
Copy link
Collaborator

kartben commented Nov 21, 2023

@kartben - does it look OK for now? These POSIX doc changes were meant to be a backportable precursor to additional features in POSIX and POSIX configuration (which probably cannot be backported).

mostly ok yes, thanks! I just added one more comment + you need a minor change to fix the currently broken build

Revise the structure of the POSIX API docs. This separates
related items out to dedicated pages. Further improvements
could yet be made - e.g. using the 'collapse' feature to
expand and collapse large sections of text or tables.

Signed-off-by: Christopher Friedt <[email protected]>
@cfriedt cfriedt force-pushed the posix-relnotes-extra-3.5.0 branch from eab576d to 9af282d Compare November 22, 2023 20:23
@cfriedt cfriedt requested a review from kartben November 22, 2023 20:32
@cfriedt cfriedt merged commit 3b45235 into zephyrproject-rtos:main Nov 23, 2023
14 checks passed
@cfriedt cfriedt deleted the posix-relnotes-extra-3.5.0 branch November 23, 2023 03:47
@cfriedt cfriedt mentioned this pull request Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CMSIS API Layer CMSIS-RTOS API Layer area: Documentation Infrastructure area: Documentation area: Portability Standard compliant code, toolchain abstraction area: POSIX POSIX API Library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants