Skip to content

Fix #82: Providing the motivation and how to use this guide#281

Open
calcium-wq wants to merge 1 commit into
piotrwitek:masterfrom
calcium-wq:githubbiz/fix-issue-82
Open

Fix #82: Providing the motivation and how to use this guide#281
calcium-wq wants to merge 1 commit into
piotrwitek:masterfrom
calcium-wq:githubbiz/fix-issue-82

Conversation

@calcium-wq

Copy link
Copy Markdown

Closes #82

Automated fix for bounty issue ($50 USD).

Fix for #82

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the project's onboarding documentation by adding descriptive sections to the README and playground documentation regarding the guide's scope and usage. It also introduces a new test suite to verify README content and updates the package.json scripts, notably disabling the doctoc automation. Feedback highlights a contradiction in the doctoc setup where markers and dependencies remain despite the script being disabled. Furthermore, it is recommended to use more resilient regex patterns in the new tests to avoid brittleness against minor documentation changes.

Comment thread package.json
"doctoc": "doctoc --maxlevel=3 README_SOURCE.md",
"test": "node --test test/*.test.js",
"ci-check": "npm run readme:generate && npm test",
"doctoc": "node -e \"console.log('README_SOURCE.md TOC is maintained in source for this repository.')\"",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The doctoc script has been replaced with a log message, effectively disabling automatic Table of Contents generation. However, the README.md and README_SOURCE.md files still contain the <!-- START doctoc --> markers and instructions to run doctoc for updates. This creates a contradiction for contributors. Additionally, the doctoc package remains in devDependencies. If manual maintenance is the goal, please remove the markers and the dependency. If not, consider restoring the script's functionality.

Comment on lines +9 to +22
test('README explains what the guide is and is not', () => {
assert.match(
readme,
/### \*\*What This Guide Is\*\*/
);
assert.match(
readme,
/This repository is a reference guide and recipe collection for using React, Redux, and TypeScript together\./
);
assert.match(
readme,
/It is \*\*not\*\* a boilerplate, starter kit, or library that introduces its own replacement APIs for React\./
);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This test matches long, exact strings from the README, making it brittle to minor documentation changes (e.g., rephrasing or typo fixes). Consider using more resilient matches for key phrases or headers to improve maintainability.

test('README explains what the guide is and is not', () => {
  assert.match(readme, /### \*\*What This Guide Is\*\*/);
  assert.match(readme, /reference guide and recipe collection/);
  assert.match(readme, /not\*\* a boilerplate/);
});

Comment on lines +24 to +45
test('README gives beginners a reading path and clarifies the styleguide', () => {
assert.match(
readme,
/### \*\*How to Use This Guide\*\*/
);
assert.match(
readme,
/Start with the \[Playground Project\]\(#playground-project\) to see where the examples live\./
);
assert.match(
readme,
/### \*\*Understanding the Playground and Style Guide\*\*/
);
assert.match(
readme,
/The Styleguidist demo is a separate view over the same source files\./
);
assert.match(
readme,
/It renders markdown docs and live examples from `playground\/src`, organized by `playground\/styleguide\/styleguide\.config\.js`\./
);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Matching exact sentences from the README is prone to failure upon minor edits. Using partial matches for essential keywords or structural elements (like headers) would be more robust.

test('README gives beginners a reading path and clarifies the styleguide', () => {
  assert.match(readme, /### \*\*How to Use This Guide\*\*/);
  assert.match(readme, /Start with the \[Playground Project\]/);
  assert.match(readme, /### \*\*Understanding the Playground and Style Guide\*\*/);
  assert.match(readme, /Styleguidist demo is a separate view/);
  assert.match(readme, /organized by `playground\/styleguide\/styleguide\.config\.js`/);
});

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.

Providing the motivation and how to use this guide

1 participant