-
-
Notifications
You must be signed in to change notification settings - Fork 44
Linter feature: checking broken links and show in PR #345
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: main
Are you sure you want to change the base?
Linter feature: checking broken links and show in PR #345
Conversation
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.
Summary of Changes
Hello @anufdo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented a new linter feature to automatically detect and report broken links within markdown files. This enhancement ensures the accuracy and reliability of external and internal links throughout the documentation and other markdown content. The changes include adding a dedicated configuration file for the link checker, providing detailed documentation on its usage and integration, and updating project dependencies and scripts to support this new functionality.
Highlights
- New Link Checker Configuration: I've introduced a new
.markdown-link-check.json
configuration file. This file specifies how the link checker should operate, including patterns to ignore (like localhost and mailto links), rules for converting relative paths to absolute URLs, and settings for handling HTTP requests such as user-agents, timeouts, and retries for rate-limited responses. - Detailed Link Checker Documentation: I've added a comprehensive
docs/LINK_CHECKER.md
file. This document serves as a guide for understanding, using, and troubleshooting the new link checking feature. It covers everything from local development commands to GitHub Actions integration and best practices for maintaining link integrity. - Dependency and Script Updates: I've updated
package.json
andpackage-lock.json
to include themarkdown-link-check
package as a development dependency. This also involved adding newnpm
scripts,check-links
andcheck-links-verbose
, which allow for easy local execution of the link checker across all markdown files in the repository.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces a link checker feature using markdown-link-check
. It adds the necessary configuration, documentation, and scripts to run the checker. The configuration for handling retries and ignoring specific patterns is well-thought-out. However, I've identified a few areas for improvement. The scripts in package.json
are not cross-platform compatible, which could cause issues for developers on Windows. Additionally, the new documentation in LINK_CHECKER.md
references GitHub workflow files that are not part of this PR and contains an incomplete configuration snippet, which could lead to confusion.
package.json
Outdated
"check-links": "find . -name '*.md' -not -path './node_modules/*' -not -path './.git/*' | xargs markdown-link-check --config .markdown-link-check.json", | ||
"check-links-verbose": "find . -name '*.md' -not -path './node_modules/*' -not -path './.git/*' | xargs markdown-link-check --config .markdown-link-check.json --verbose" |
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 check-links
and check-links-verbose
scripts use find
and xargs
, which are not available on all operating systems (e.g., Windows CMD). The markdown-link-check
tool supports glob patterns directly, which is more portable and simpler. The tool also ignores the node_modules
directory by default.
"check-links": "markdown-link-check --config .markdown-link-check.json \"**/*.md\"",
"check-links-verbose": "markdown-link-check --config .markdown-link-check.json --verbose \"**/*.md\""
docs/LINK_CHECKER.md
Outdated
## GitHub Actions Integration | ||
|
||
Two workflows are available: | ||
|
||
### 1. Custom Workflow (`.github/workflows/link-check.yml`) | ||
|
||
- Runs on pull requests that modify markdown files | ||
- Uses the same configuration as local development | ||
- Comments on PRs when broken links are found | ||
- Provides detailed error information | ||
|
||
### 2. Lychee Action (`.github/workflows/link-check-action.yml`) | ||
|
||
- Alternative implementation using `lychee-action` | ||
- Simpler setup but less customizable | ||
- Also runs on markdown file changes |
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 documentation mentions two GitHub Actions workflows (.github/workflows/link-check.yml
and .github/workflows/link-check-action.yml
) that are not included in this pull request. This could be confusing for future readers.
It's better to either include these files in this PR or remove the documentation for them until they are added.
docs/LINK_CHECKER.md
Outdated
```json | ||
{ | ||
"ignorePatterns": [ | ||
{"pattern": "^http://localhost"}, | ||
{"pattern": "^https://localhost"}, | ||
{"pattern": "^#%"}, | ||
{"pattern": "^mailto:"} | ||
], | ||
"replacementPatterns": [ | ||
{ | ||
"pattern": "^/", | ||
"replacement": "https://brisbanesocialchess.github.io/" | ||
} | ||
], | ||
"aliveStatusCodes": [200, 206, 429], | ||
"timeout": "15s", | ||
"retryOn429": true, | ||
"retryCount": 3 | ||
} | ||
``` |
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 configuration reference snippet is incomplete. It's missing the httpHeaders
and fallbackRetryDelay
sections from the actual .markdown-link-check.json
file.
To avoid confusion and keep the documentation in sync with the configuration, it would be better to either make the snippet complete or simply refer to the configuration file itself. Here is the complete configuration from .markdown-link-check.json
:
{
"ignorePatterns": [
{
"pattern": "^http://localhost"
},
{
"pattern": "^https://localhost"
},
{
"pattern": "^#%"
},
{
"pattern": "^mailto:"
}
],
"replacementPatterns": [
{
"pattern": "^/",
"replacement": "https://brisbanesocialchess.github.io/"
}
],
"httpHeaders": [
{
"urls": ["https://github.com"],
"headers": {
"Accept": "text/html",
"User-Agent": "Mozilla/5.0"
}
}
],
"aliveStatusCodes": [200, 206, 429],
"timeout": "15s",
"retryOn429": true,
"retryCount": 3,
"fallbackRetryDelay": "30s"
}
Good idea. thanks. please review and check google geminai feedback and answer on each comment. many thanks. |
Note: also we need to run it inside pre-commit as well. |
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.
We need to do this with pre-commit
.
We already have pre-commit
running with GitHub Actions.
The other choice is to use markdown-link-check. @BaseMax is a maintainer there
@anufdo please continue and do same checking process in pre-commit too. |
Please pull new changes, and fix all cicd issues, you have dprint, pre-commit errors now. |
Many thanks in advance. |
…y invocation in npm scripts\n- Add minimal GITHUB_TOKEN permissions to workflows\n- Update link-check workflow to use npx with directory\n- Sync docs with full config (headers + fallback delay)\n- Ensure pre-commit includes markdown-link-check hook
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.
Code Review
This pull request introduces a valuable feature by adding a markdown link checker to the pre-commit hooks, which will help maintain link integrity across the project. The configuration for the checker and the integration into package.json
and .pre-commit-config.yaml
are well-implemented. I have a couple of suggestions for the new documentation file (LINK_CHECKER.md
) to improve its accuracy and maintainability by removing references to non-existent workflows and avoiding duplication of the configuration file.
Two workflows are available: | ||
|
||
### 1. Custom Workflow (`.github/workflows/link-check.yml`) | ||
|
||
- Runs on pull requests that modify markdown files | ||
- Uses the same configuration as local development | ||
- Comments on PRs when broken links are found | ||
- Provides detailed error information | ||
|
||
### 2. Lychee Action (`.github/workflows/link-check-action.yml`) | ||
|
||
- Alternative implementation using `lychee-action` | ||
- Simpler setup but less customizable | ||
- Also runs on markdown file changes | ||
|
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.
This section describes two GitHub Actions workflows that don't seem to be part of this pull request or the repository yet. Documenting functionality that doesn't exist can be confusing for developers. It's best to keep documentation aligned with the current state of the codebase. I'd recommend removing this section for now. It can be added back in a separate pull request when the workflows are actually implemented.
frontend/LINK_CHECKER.md
Outdated
```json | ||
{ | ||
"ignorePatterns": [ | ||
{"pattern": "^http://localhost"}, | ||
{"pattern": "^https://localhost"}, | ||
{"pattern": "^#%"}, | ||
{"pattern": "^mailto:"} | ||
], | ||
"replacementPatterns": [ | ||
{ | ||
"pattern": "^/", | ||
"replacement": "https://brisbanesocialchess.github.io/" | ||
} | ||
], | ||
"httpHeaders": [ | ||
{ | ||
"urls": ["https://github.com"], | ||
"headers": { | ||
"Accept": "text/html", | ||
"User-Agent": "Mozilla/5.0" | ||
} | ||
} | ||
], | ||
"aliveStatusCodes": [200, 206, 429], | ||
"timeout": "15s", | ||
"retryOn429": true, | ||
"retryCount": 3, | ||
"fallbackRetryDelay": "30s" | ||
} | ||
``` |
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.
This section duplicates the entire content of .markdown-link-check.json
. This is not ideal for maintainability, as changes to the JSON file will require a parallel change here to keep the documentation in sync. This violates the DRY (Don't Repeat Yourself) principle. It's better to avoid this duplication and simply refer to the configuration file, as you've already done in the preceding sentence.
…down link handling
scripts/check-links.js
Outdated
} | ||
|
||
try { | ||
execSync(cmd, { stdio: 'inherit' }); |
Check warning
Code scanning / CodeQL
Shell command built from environment values
…lidate file paths and avoid shell execution
What is happening here ? There are conflicts. It has been almost a month ? |
No description provided.