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

Add table of contents to README for better navigation #364

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

Dhie-boop
Copy link
Contributor

What does this PR do?

  • Adds a table of contents to the README file.
  • Links to all major sections for easy navigation.

Why is this change needed?

  • To improve the user experience for contributors and readers.
  • Makes it easier to find relevant information quickly.

Checklist:

  • Table of contents added with links.
  • Verified that all links work correctly.
  • Proofread for clarity.

Let me know if any changes are needed!

Copy link

@Ndegwadavid Ndegwadavid left a comment

Choose a reason for hiding this comment

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

Good format :-)

@Dhie-boop
Copy link
Contributor Author

Thanks! I'm glad the format is clear. Let me know if you have any suggestions or further feedback! 😊

@Dhie-boop Dhie-boop requested a review from Ndegwadavid January 27, 2025 11:41
@Dhie-boop Dhie-boop requested a review from walex999 January 27, 2025 13:26
Copy link

@moktermd08 moktermd08 left a comment

Choose a reason for hiding this comment

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

looks okay to me

Copy link
Contributor

@jacksonpradolima jacksonpradolima left a comment

Choose a reason for hiding this comment

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

@Dhie-boop thanks for your contribution! 🚀

GH already provides TOC natively:
image

Besides that, it's missing subsections.

@Dhie-boop
Copy link
Contributor Author

@jacksonpradolima Thank you for the feedback and the clarification! 😊 I now see that GitHub’s native TOC feature already serves this purpose.
If my changes are no longer necessary, feel free to close this PR. Otherwise, let me know if there’s anything else I can adjust or improve.

Thanks again for reviewing and guiding me!

Copy link

@haciz haciz left a comment

Choose a reason for hiding this comment

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

this could also be merged, into don't see reason not to, basic changes for better readability.

Copy link

@Ndegwadavid Ndegwadavid left a comment

Choose a reason for hiding this comment

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

Seems good to me

Copy link

@erickcestari erickcestari left a comment

Choose a reason for hiding this comment

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

The original format used ## which is incorrect for linking to headings. In markdown, headings are linked using # followed by the heading text in lowercase, with spaces replaced by hyphens.

Correct version:

1. [Introduction](#1-introduction)  
2. [Model Summary](#2-model-summary)  
3. [Model Downloads](#3-model-downloads)  
4. [Evaluation Results](#4-evaluation-results)  
5. [Chat Website & API Platform](#5-chat-website--api-platform)  
6. [How to Run Locally](#6-how-to-run-locally)  
7. [License](#7-license)  
8. [Citation](#8-citation)  
9. [Contact](#9-contact)  

==

  1. Introduction
  2. Model Summary
  3. Model Downloads
  4. Evaluation Results
  5. Chat Website & API Platform
  6. How to Run Locally
  7. License
  8. Citation
  9. Contact

@Dhie-boop
Copy link
Contributor Author

@erickcestari Thanks for pointing this out! I’ve updated the TOC to use the correct Markdown format for linking to headings.

Copy link

@Ndegwadavid Ndegwadavid left a comment

Choose a reason for hiding this comment

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

seems good

@mowentian mowentian merged commit c32c957 into deepseek-ai:main Feb 5, 2025
@dinithaw
Copy link

dinithaw commented Feb 5, 2025

Looking good 👍

@musvaage
Copy link

musvaage commented Feb 8, 2025

@mowentian (apologies for commenting on this closed pull)

IMHO, the decision to merge was short sighted.

This pertains at least to this Organization's repository.

cf: DeepSeek_V3.pdf

Where the README.md file's 1. Introduction and 2. Model Summary, respectively are based on the .pdf file's Abstract and Introduction, and as html <h1> and <h2> elements', indeed and markdown format # and ##, effects are evident, there does not seem to be any purpose for retaining the enumeration of the README.md file's sections/subsections.

As below the ToC has a poor æsthetic, and if this pull were eventually reverted, thereafter might be considered removing the enumeration of the README.md file's sections/subsections.

Obviously, enumeration of the README.md file's sections/subsections occurs in [Organization] sibling repositories.

DeepSeek-R1

cf: https://github.com/deepseek-ai/DeepSeek-V3/pull/364#pullrequestreview-2576152035

The GH native Outline accessible via the button button is unobtrusive.

IMHO, an indented numbered list has a poor æsthetic.

Organization wide standardization wasn't a subject when DeepSeek-V3 #364 was under discussion.

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.

10 participants