Skip to content

Conversation

@mohithshuka
Copy link

@mohithshuka mohithshuka commented Jan 15, 2026

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.
  • I have mentioned this change in the changelog.

My familiarity with the project is as follows (check one):

  • I have never used CCExtractor.
  • I have used CCExtractor just a couple of times.
  • I absolutely love CCExtractor, but have not contributed previously.
  • I am an active contributor to CCExtractor.

{pull request content here}

Copy link
Contributor

@cfsmp3 cfsmp3 left a comment

Choose a reason for hiding this comment

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

Thank you for working on improving the README! However, there are a few issues that need to be addressed before this can be merged.

Issues Found

1. Typo Introduced

The PR accidentally removes the word "a" from an existing sentence:

  • Before: "subtitles are a great way to learn"
  • After: "subtitles are great way to learn"

This is grammatically incorrect.

2. Broken Markdown

The code block in the Quick Start section is not properly closed - it's missing the closing ```. This will break the README rendering.

3. Redundant Content

The same command already exists in the README at lines 50-54:

Extracting subtitles is relatively simple. Just run the following command:
`ccextractor <input>`
This will extract the subtitles.

Adding it again in a Quick Start section creates duplication.

4. Document Flow

The Quick Start section is inserted between "Installation and Usage" and "Windows Package Managers", which breaks the logical hierarchy. The Windows Package Managers section was meant to be grouped under Installation.

Suggestions

Option A: Keep it simple - just submit the repository link formatting fix (which is good!) as a separate small PR, and drop the Quick Start section.

Option B: If you want to add a Quick Start section, consider:

  • Placing it right after the introduction (before Installation)
  • Including BOTH installation and basic usage
  • Removing the duplicate content from below
  • Making sure it's properly formatted

What's Good

The repository link change is genuinely better:

  • Removes unnecessary parentheses
  • Uses proper punctuation
  • Formats master as inline code

Please fix these issues and I'll be happy to review again!

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.

2 participants