Skip to content
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

Port Lychee link checker to user documentation #531

Merged
merged 5 commits into from
Oct 17, 2024

Conversation

Tridecatrix
Copy link
Contributor

@Tridecatrix Tridecatrix commented Oct 17, 2024

Ported the Lychee link checker to user documentation as indicated here:
#526 (comment)

Example output of Lychee from running the workflow on my local branch
https://github.com/Tridecatrix/user-documentation/actions/runs/11379543940/attempts/1#summary-31657176353

Pretty much just copy pasted over the check-links.yml file and .lycheeignore for now. From a quick check of a selection of the links given by Lychee, it does seem to work, i.e. the links it says are broken are really broken. Except for one example I'll indicate further below.

Require feedback on:

  • Whether other links should be added to .lycheeignore
    • (it seems like the links in there currently are things like www.arxiv.org, i.e. links to global resources corresponding to journal entries and the like. I think the rationale behind ignoring these were to allow including bogus journal entry examples in documentation like www.arxiv.org/fake-journal-entry without triggering the links checker? There are a few links to global resources that are in the above linked error output that correspond to hyperrefs to additional support resources like AutoHotKey.com however. We want links to support resources to be valid I think. I'd like some clarification on the rationale for including things in lycheeignore

As for that one example of a bad detection, it was this, which indicates a broken reference:
image

Even though the original code has none:
image

@koppor
Copy link
Member

koppor commented Oct 17, 2024

We had the same idea at the same time 😅🙈 --> #532

I merged the two workflows to make it easier for contributors. WDYT? We can also split them again ^^

@koppor
Copy link
Member

koppor commented Oct 17, 2024

  • . I'd like some clarification on the rationale for including things in lycheeignore

In case they use CloudFlare and we think the links are stable, we add it to .lycheeignore. You can try to remove all ignored and see the output of the checker. Mabe, it got better the last years. -- The main reason was that we accept bad links and see them reported/fixed by users instead of be weekly reminded that this spam protection causes effort on our side. - We have constraint resources and would rather like to work on the tool and documentation itself than investing time in getting links checkers running properly.

@Tridecatrix
Copy link
Contributor Author

Tridecatrix commented Oct 17, 2024

Woah! Crazy.
Yeah okay fair enough re: lycheeignore.

Sidenote: I have also been trying to set up the workflow to copy the check-links.yaml from the main Jabref/jabref repo when updated but I am severely limited by my GitHub workflow knowledge unfortunately, doesn't seem feasible for me given my time constraints at the moment lol. Chalk it up as an issue I guess.

Here's the little I achieved (in new workflow called sync-check-links.yaml, not actually included in this PR because not nearly complete enough to push).

name: Sync check-links.yaml with the version on the main Jabref repo.
on:
  workflow_dispatch:

# Resources: 
# - https://stackoverflow.com/questions/68057744/create-pull-request-with-github-action 
# - https://www.paigeniedringhaus.com/blog/copy-files-from-one-repo-to-another-automatically-with-git-hub-actions

jobs:
  sync-check-links:
    runs-on: ubuntu-latest
    steps:
      # TODO: Update below to only conditionally execute the action

      - name: Create new branch auto-sync
      # TODO

      - name: Copy file from JabRef repo into user-documentation/auto-sync  
      # TODO. Potentially use https://github.com/marketplace/actions/copy-files-to-another-repository

      - name: Create pull request
        run: gh pr create -B auto-sync -H bmain --title 'Sync check-links.yaml from main JabRef' --body 'Created by Github action'
        env:
            GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

@Tridecatrix
Copy link
Contributor Author

Re: splitting or combining the workflows I suppose what you lose is the ability to change the conditions for executing (i.e. the on field) for each workflow seperately?

@koppor
Copy link
Member

koppor commented Oct 17, 2024

Re: splitting or combining the workflows I suppose what you lose is the ability to change the conditions for executing (i.e. the on field) for each workflow seperately?

Yes! I was accepting that th emarkdown linter is running also periodically. All other triggers are the same. -- You can split the workflows again if you think its better -- because also the "play" button in the actions tab could be more intuitive.

@Tridecatrix
Copy link
Contributor Author

My two cents are I would keep it split as I think one workflow per seperate check looks nicer anyway. :)

@koppor
Copy link
Member

koppor commented Oct 17, 2024

I'll merge your PR then.

@koppor koppor marked this pull request as ready for review October 17, 2024 07:49
@koppor
Copy link
Member

koppor commented Oct 17, 2024

Oh, I cannot merge, because there are two workflows for link checking now:

image

It should be one!

@Tridecatrix
Copy link
Contributor Author

Sorry, removed the second one!

@koppor
Copy link
Member

koppor commented Oct 17, 2024

You also need to remove th mlc_config.json in the root.

@koppor koppor merged commit 3f20765 into JabRef:main Oct 17, 2024
1 check passed
@Tridecatrix
Copy link
Contributor Author

Tridecatrix commented Oct 17, 2024

Thanks! Nice. Will now work on fixing the links a-la #533 :).

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