-
Notifications
You must be signed in to change notification settings - Fork 76
Update page for DDK 25.2 release #559
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
base: master
Are you sure you want to change the base?
Conversation
|
I do not think these links are needed, but if others want them they should at least be anonymous (two underscores at the end). |
The links are a nice addition to the docs. @ssurt2 I do agree with @StaticRocket on the anonymous links. Since these links targets are only used once. |
|
@ssurt2 the commit title is a bit vague. Maybe: style(Graphics): Add links to all supported extensions ? Change it however you like you tho . |
|
@ssurt2 , you have chosen poorly... |
Antonios-C
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit of a nit pick, commit message mentions "Overview" when all commit diffs are not on the overview.rst, but instead overview.rst includes the extension files. I wouldn't ask for another revision.
This looks good to me. Thank you for your help. Great job Shriya on your first 1st contribution to the docs !
|
New warnings found with rstcheck: |
source/linux/Foundational_Components/Graphics/Rogue/_8XE_Extension_List.rst
Outdated
Show resolved
Hide resolved
cshilwant
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ssurt2 please plan to handle - #559 (comment)
StaticRocket
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double check those few IMG links. If there's a proper spec definition I want people to see that first. We should only link to IMG docs if there really is no other information, otherwise people might incorrectly assume this is some IMG specific thing.
source/linux/Foundational_Components/Graphics/Rogue/_BXS_Extension_List.rst
Outdated
Show resolved
Hide resolved
source/linux/Foundational_Components/Graphics/Rogue/_BXS_Extension_List.rst
Outdated
Show resolved
Hide resolved
source/linux/Foundational_Components/Graphics/Rogue/_BXS_Extension_List.rst
Outdated
Show resolved
Hide resolved
Antonios-C
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit title and message need to be changed.
You are incorporating feedback you have received on previous revisions of this PR within the commit message. If you do want to list and track changes between revisions follow this guide (specifically the section regarding patch changelogs).
From an outside perspective (someone that hasn't been following this PR), if they read the commit message they will get confused by "remove link for EGL_IMG_cl_image", as there was no link to begin with.
I suggest your commit to look something like this:
feat(Graphics): Add hyperlinks to all supported extensions
Add hyperlinks to all supported extensions. Add note regarding any missing links.
signed-off ......
Add anonymous hyperlinks to all supported extensions. Add note section for extensions without links. Signed-off-by: Shriya Surti <a0511311@ti.com>
| .. note:: | ||
|
|
||
| All extensions without links are custom IMG extensions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation here is incorrect. Additionally, the GL_IMG_texture_format_BGRA8888 and GL_IMG_texture_npot are still defined. They should be dropped and those sections should have a similar note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GL_IMG_texture_format_BGRA8888 link: https://registry.khronos.org/OpenGL/extensions/EXT/EXT_texture_format_BGRA8888.txt
style(Graphics):Overview: Updated page for DDK 25.2 release.
Added links for the extensions we support.