-
-
Notifications
You must be signed in to change notification settings - Fork 402
fix(ci): fix link-checker context and prevent deploy on PR #176
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?
Conversation
|
Hello @ItsCurulo , can you confirm that no AI was used in this pull request? The wording of the pull request and the solution you have proposed have the 'feel' of AI The solution does not implement a fix for issue #145 - the fix must be in the pull request pipepline |
jgadsden
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 fix needs to be in the pull request pipeline not the commit pipeline
|
Hello @jgadsden, thank you for the feedback. |
| uses: DavidAnson/markdownlint-cli2-action@v22.0.0 | ||
| with: | ||
| config: '.markdownlint.yaml' | ||
| config: ".markdownlint.yaml" |
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.
no need for a change to double quotes here, as no escape sequences are expected, so can be reverted
| with: | ||
| python-version: '3.10' | ||
| python-version: "3.10" | ||
|
|
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.
same comment as for line 49
| name: Deploy web doc | ||
| runs-on: ubuntu-latest | ||
|
|
||
| needs: [md_linter, spell_checker] |
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.
ne need for new line
| uses: DavidAnson/markdownlint-cli2-action@v22.0.0 | ||
| with: | ||
| config: '.markdownlint.yaml' | ||
| config: ".markdownlint.yaml" |
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.
as above, no need for double quotes as there are no escape sequences, same for line 86
| - name: Checkout markdown | ||
| uses: actions/checkout@v6.0.1 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} |
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 is implied by the pull request action and so is not needed
jgadsden
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.
it would be good to implement this by excluding the 'blob' links that are actually causing the failures when .md files are added / deleted within a pull request
see lycheedocs for how to do this, along with the .lycheeignore file
Summary :
This PR addresses issue #145 by updating the CI pipeline configuration to correctly handle Pull Requests.
Currently, the link checker defaults to the
mainbranch or fails to trigger on PRs, leading to false positives/negatives. This fix ensures the workflow scans the actual code being proposed in the PR.Changes
pull_requesttrigger toci.yaml.link_checkerjob to useref: ${{ github.event.pull_request.head.sha || github.sha }}. Ths targets the specific commit of the PR.if: github.event_name == 'push' ...) to thedeployjob. This prevents the pipeline from attempting to deploy the documentation website during a PR check.Closes #145
Description for the changelog :
Fixed CI pipeline configuration to correctly handle Link Checker and deployments on Pull Requests.
Declaration:
Other info :
Tested in a forked repository. The workflow successfully triggered on the Pull Request and scanned the files as expected.
Output summary from the test run: