ci: added a link-check ci #297

Gmin2 · 2024-02-08T16:06:41Z

Fixes #103

follow up to #291

Relequestual · 2024-02-09T12:54:55Z

Thanks for your contribution! =]

I think this is blocked by #295

If we merged this before actually fixing links, it would block PRs till the bad links are fixed, I think.

Gmin2 · 2024-02-09T13:28:58Z

If we merged this before actually fixing links, it would block PRs till the bad links are fixed, I think.

Lets wait for #295 to be solved

DarhkVoyd · 2024-02-09T13:37:14Z

Hey, I am not sure if linkinator is the right tool for checking broken links for our use-case.

Gmin2 · 2024-02-09T13:38:32Z

Hey, I am not sure if linkinator is the right tool for checking broken links for our use-case.

reason will be appreciated @DarhkVoyd

Gmin2 · 2024-02-11T10:39:26Z

Hey @Relequestual can you review it made some changes here

pages/blog/posts/bowtie-intro.md

.lycheeignore

.github/workflows/link-check.yml

Relequestual

Moving in the right direction. Made a few comments for change. Thanks for your continued effort on this work! =D

.github/workflows/link-check.yml

Gmin2 · 2024-02-23T06:50:43Z

@Relequestual made some changes a review will be appreciated and left some comment in the code

benjagm

We very close. Nice job!

Left some comment.

.lycheeignore

pages/blog/posts/bowtie-intro.md

.github/workflows/link-check.yml

benjagm

I havent tested it but looks good to me.

pages/blog/posts/bowtie-intro.md

aialok · 2024-03-04T11:51:07Z

Thank you for your changes, @utnim2. To ensure that the Build Job doesn't fail, we need to include an additional line to fetch submodules during the repository checkout process. Adding the following line will resolve the issue:

- name: Checkout repository
  uses: actions/checkout@v4
  with:
    submodules: recursive

Checkout this for more details : #297 (comment)

aialok · 2024-03-04T12:23:41Z

.github/workflows/link-check.yml

+      - name: Build Next.js App
+        run: yarn build
+
+      - name: Serve App Locally


I think you need to remove this steps and add this to linkchecker job as we need our app to run locally so that linkchecker can do it job.

Suggested change

- name: Serve App Locally

Remove this step

Here no need to run app locally.

Thanks for your review @aialok

no we need to first build and deploy it here and pass the deployed url in the linkchecker action (i have put localhost:3000 as of now)

The deploy step is missing here as the deployment is not yet clear

@aialok

Yes, I too agree with you regarding to need to build and deploy locally.

Thanks for your review @aialok

no we need to first build and deploy it here and pass the deployed url in the linkchecker action (i have put localhost:3000 as of now)

The deploy step is missing here as the deployment is not yet clear

@aialok

Got it man. But I still have one doubt what do you mean by deploy locally. Deploy locally means running on localhost?

Thank you @utnim2. You have done pretty good job : )

aialok · 2024-03-04T12:26:58Z

.github/workflows/link-check.yml

+        uses: actions/checkout@v4
+        with:
+          submodules: recursive
+


Add steps to run our app locally, as we need it to run locally since most of our links are based on localhost.

see comment here

aialok

@utnim2 , See Comment : )

aialok · 2024-03-04T13:58:22Z

With reference to this comment : #297 (comment)

I think we do already deploy our app on cloudflare.
We can directly use https://json-schema.org/ instead of this http://localhost:3000 as suggested in comment: #297 (comment)

@utnim2 @DarhkVoyd

DarhkVoyd · 2024-03-04T14:01:52Z

With reference to this comment : #297 (comment)

I think we do already deploy our app on cloudflare. We can directly use https://json-schema.org/ instead of this http://localhost:3000 as suggested in comment: #297 (comment)

@utnim2 @DarhkVoyd

Please refer to #297 (comment)

aialok · 2024-03-04T14:13:17Z

With reference to this comment : #297 (comment)
I think we do already deploy our app on cloudflare. We can directly use https://json-schema.org/ instead of this http://localhost:3000 as suggested in comment: #297 (comment)
@utnim2 @DarhkVoyd

Please refer to #297 (comment)

My Bad...Got it : )

DarhkVoyd

Hello @benjagm @utnim2 @aialok

This PR was quite difficult and posed some challenges during review. So, please bear with me if I make some mistakes and I appreciate your patience as I worked through it. Below are my findings, change requests, and suggestions:

We should consider either merging the two jobs or rebuilding and serving the website again in the second job to address post-action cleanup in the workflow.
Despite the expected downtime of our served website due to post-cleanup actions, the initial job fails to serve the website. The current workflow experiences a silent failure in the first job due to a command not found error when serving the website.
Upon investigation, it appears that the serve package, utilized in our package.json for local website deployment, is missing from our dependencies. This absence is the root cause of the deployment failure. To resolve this issue, we must ensure the inclusion of the serve package in our dependencies.
Given that we use localhost as the base for our action, we need to define which files should be considered part of the action. Because, I am not sure if all markdown file references are served on our website this ensures we avoid unnecessary errors.
In my testing of the workflow, I found several broken links. To prevent workflow failure, I suggest creating a separate issue to address these broken links before merging this pull request. The broken links may be related to unnecessary files from submodules or other issues discussed in point 4. We might also use .lycheeignore to ignore certain URIs or URLs.

Thank you for your patience. For a more detailed overview of my changes, please visit this link.

DarhkVoyd · 2024-03-06T06:23:25Z

.github/workflows/link-check.yml

+      - name: Wait for App to Start
+        run: sleep 20
+
+  linkChecker:


Changing jobs runs post action cleanup in workflow which closes all of our node processes from previous step which includes the one serving the site on local host. So, we should either build and serve again or simply merge them into a single job.

Gmin2 · 2024-03-06T12:08:40Z

@DarhkVoyd thanks for you review
Can you refer this comment about the deploy part

Gmin2 · 2024-03-06T12:09:57Z

Can you add your opinion @Relequestual

benjagm · 2024-03-06T12:24:19Z

Can you add your opinion @Relequestual

Can we just validate the external links for now and leave the internal ones for a second iteration?
Reference here

benjagm · 2024-03-06T12:28:02Z

An alternative approach can be pause this effort to validate broken links in deployment time, to use a different approach to have a crawler finding broken links every X days. For example:

https://github.com/marketplace/actions/broken-links-crawler

Specifically I like this approach: https://andrewwegner.com/find-broken-links-with-github-actions.html

DarhkVoyd · 2024-03-06T12:28:33Z

@DarhkVoyd thanks for you review Can you refer this comment about the deploy part

utnim2,
I believe you have misinterpreted my suggestion.

aialok · 2024-03-06T12:43:03Z

Hello @benjagm @utnim2 @aialok

This PR was quite difficult and posed some challenges during review. So, please bear with me if I make some mistakes and I appreciate your patience as I worked through it. Below are my findings, change requests, and suggestions:

We should consider either merging the two jobs or rebuilding and serving the website again in the second job to address post-action cleanup in the workflow.

Despite the expected downtime of our served website due to post-cleanup actions, the initial job fails to serve the website. The current workflow experiences a silent failure in the first job due to a command not found error when serving the website.
Upon investigation, it appears that the serve package, utilized in our package.json for local website deployment, is missing from our dependencies. This absence is the root cause of the deployment failure. To resolve this issue, we must ensure the inclusion of the serve package in our dependencies.

Given that we use localhost as the base for our action, we need to define which files should be considered part of the action. Because, I am not sure if all markdown file references are served on our website this ensures we avoid unnecessary errors.

In my testing of the workflow, I found several broken links. To prevent workflow failure, I suggest creating a separate issue to address these broken links before merging this pull request. The broken links may be related to unnecessary files from submodules or other issues discussed in point 4. We might also use .lycheeignore to ignore certain URIs or URLs.

Thank you for your patience. For a more detailed overview of my changes, please visit this link.

Great review! Thanks, @DarhkVoyd.

I've gone through your workflow, and everything looks good to me. However, I noticed that we are checking links of .md files in node_modules, which need to be ignored as they are causing thousands of links to be checked unnecessarily.

Other than this, everything looks good to me.

@benjagm, your solution is also a good way to handle things.

Happy coding!

DarhkVoyd · 2024-03-06T13:08:03Z

Great review! Thanks, @DarhkVoyd.
I've gone through your workflow, and everything looks good to me. However, I noticed that we are checking links of .md files in node_modules, which need to be ignored as they are causing thousands of links to be checked unnecessarily.

aialok,
Thank you so much for the review. I too was confused about several unnecessary files coming up in the workflow but could not understand the origination as expressed in my review.

Can we just validate the external links for now and leave the internal ones for a second iteration?

benjagm,
I agree with your solution and I believe that it is the only correct implementation for this workflow. Since, we want to maintain our relative paths in the markdown for relative links we probably will have to break down our types of links not only into external and internal but into relative paths too.

An alternative approach can be pause this effort to validate broken links in deployment time, to use a different approach to have a crawler finding broken links every X days.

This is another great solution, however we might still end up with our pursuit of errors related to external and internal links.

Gmin2 · 2024-03-06T13:11:43Z

Can we just validate the external links for now and leave the internal ones for a second iteration? Reference here

I too agree with this

Relequestual · 2024-04-02T12:24:10Z

Can we just validate the external links for now and leave the internal ones for a second iteration? Reference here

I too agree with this

I'm not concerned much about external links, as they can be broken any time. I'm much more concerned about internal links. We should avoid introducing changes which cause internal links to break. That should be our primary objective here (at least to me).

Relequestual

Let's avoid checking the node_modules folder =] .

Is it possible to only check internal links?
I don't mind also checking external links, but I don't think it's as needed or the intent of the Issue.

Gmin2 · 2024-04-02T19:32:47Z

Let's avoid checking the node_modules folder =] .

Is it possible to only check internal links? I don't mind also checking external links, but I don't think it's as needed or the intent of the Issue.

So i need to modify the action so that it checks only for external links right?

cc @benjagm @Relequestual

benjagm · 2024-04-11T09:05:52Z

So i need to modify the action so that it checks only for external links right?

What Ben was saying is check only internal but the problem with internal links but as we have been discussing this approach seems to not work for the specific type of architecture and stack of this site.

benjagm · 2024-04-15T05:43:45Z

Closing this as there was not activity for the last weeks.

added a link-check ci

94cbe48

Gmin2 added 7 commits February 11, 2024 15:03

replace likinator with linkchecker

19ec069

added .lyncheeignore

49c043b

added the path for the linkchecker action

dc2a763

cleanup

8ddd864

triggering the workflow

01dee2c

added a comment on github if the workflow fails

a9f2cfd

specified the path

7607ac6

DarhkVoyd requested changes Feb 11, 2024

View reviewed changes

pages/blog/posts/bowtie-intro.md Outdated Show resolved Hide resolved

.lycheeignore Show resolved Hide resolved

.github/workflows/link-check.yml Show resolved Hide resolved

Relequestual reviewed Feb 12, 2024

View reviewed changes

.github/workflows/link-check.yml Outdated Show resolved Hide resolved

Relequestual self-requested a review February 12, 2024 09:15

Relequestual requested changes Feb 12, 2024

View reviewed changes

DarhkVoyd reviewed Feb 12, 2024

View reviewed changes

.github/workflows/link-check.yml Outdated Show resolved Hide resolved

praveen-rikhari mentioned this pull request Feb 22, 2024

Improve the CI/CD pipeline to avoid broken links #103

Closed

1 task

Gmin2 added 2 commits February 23, 2024 11:46

added the build process

3677dfe

corrected the path of the workflow

e5e76c3

Gmin2 mentioned this pull request Feb 23, 2024

Get previews for PRs from forks #330

Closed

benjagm requested changes Feb 24, 2024

View reviewed changes

JeelRajodiya mentioned this pull request Feb 28, 2024

Add a linkchecker for the front end bowtie-json-schema/bowtie#963

Open

Gmin2 requested review from benjagm and DarhkVoyd February 28, 2024 07:56

Gmin2 closed this Feb 28, 2024

Gmin2 reopened this Feb 28, 2024

benjagm approved these changes Feb 28, 2024

View reviewed changes

pages/blog/posts/bowtie-intro.md Outdated Show resolved Hide resolved

Gmin2 added 3 commits March 4, 2024 11:55

added git submodule

8f47056

cleanup

9511807

cleanups

3962bb4

aialok reviewed Mar 4, 2024

View reviewed changes

benjagm requested review from DarhkVoyd and aialok March 5, 2024 16:18

DarhkVoyd requested changes Mar 6, 2024

View reviewed changes

aialok mentioned this pull request Mar 25, 2024

Feature : Added linkChecker workflow #604

Merged

Relequestual requested changes Apr 2, 2024

View reviewed changes

Relequestual added Status: In Progress This issue is being worked on, and has someone assigned. and removed Status: Completed Nothing further to be done with this issue. labels Apr 2, 2024

benjagm closed this Apr 15, 2024

ci: added a link-check ci #297

ci: added a link-check ci #297

Conversation

Gmin2 commented Feb 8, 2024

Relequestual commented Feb 9, 2024 • edited Loading

Gmin2 commented Feb 9, 2024

DarhkVoyd commented Feb 9, 2024

Gmin2 commented Feb 9, 2024

Gmin2 commented Feb 11, 2024

Relequestual left a comment

Choose a reason for hiding this comment

Gmin2 commented Feb 23, 2024

benjagm left a comment

Choose a reason for hiding this comment

benjagm left a comment

Choose a reason for hiding this comment

aialok commented Mar 4, 2024

aialok Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

Gmin2 Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

DarhkVoyd Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

aialok Mar 4, 2024

Choose a reason for hiding this comment

aialok Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

Gmin2 Mar 4, 2024

Choose a reason for hiding this comment

aialok left a comment

Choose a reason for hiding this comment

aialok commented Mar 4, 2024

DarhkVoyd commented Mar 4, 2024

aialok commented Mar 4, 2024

DarhkVoyd left a comment • edited Loading

Choose a reason for hiding this comment

DarhkVoyd Mar 6, 2024 • edited Loading

Choose a reason for hiding this comment

Gmin2 commented Mar 6, 2024

Gmin2 commented Mar 6, 2024

benjagm commented Mar 6, 2024

benjagm commented Mar 6, 2024 • edited Loading

DarhkVoyd commented Mar 6, 2024

aialok commented Mar 6, 2024

DarhkVoyd commented Mar 6, 2024 • edited Loading

Gmin2 commented Mar 6, 2024 • edited Loading

Relequestual commented Apr 2, 2024

Relequestual left a comment

Choose a reason for hiding this comment

Gmin2 commented Apr 2, 2024

benjagm commented Apr 11, 2024

benjagm commented Apr 15, 2024

Relequestual commented Feb 9, 2024 •

edited

Loading

aialok Mar 4, 2024 •

edited

Loading

Gmin2 Mar 4, 2024 •

edited

Loading

DarhkVoyd Mar 4, 2024 •

edited

Loading

aialok Mar 4, 2024 •

edited

Loading

DarhkVoyd left a comment •

edited

Loading

DarhkVoyd Mar 6, 2024 •

edited

Loading

benjagm commented Mar 6, 2024 •

edited

Loading

DarhkVoyd commented Mar 6, 2024 •

edited

Loading

Gmin2 commented Mar 6, 2024 •

edited

Loading