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

Add Validation + Checks to markdown PRs #1538

Open
wesbos opened this issue Feb 7, 2024 · 14 comments
Open

Add Validation + Checks to markdown PRs #1538

wesbos opened this issue Feb 7, 2024 · 14 comments

Comments

@wesbos
Copy link
Collaborator

wesbos commented Feb 7, 2024

We need to create a github action that does validation on the markdown files when we PR them. This way we can merge with confidence.

I've done this in the past with my uses.tech site: https://github.com/wesbos/awesome-uses/blob/master/.github/workflows/data-validate.yml

So, things that should be checked:

  1. Publish date is in the future
  2. mp3 url is valid (Fetch with HEAD to ensure file exists)
  3. Show number is not overlapping with any existing shows
  4. All links in markdown resolve 200 OK
  5. Perhaps some sort of spellcheck?
  6. Youtube url is a real youtube url (we had them mixed up once)
  7. all timestamp links adhere to HH:MM:SS or HH:MM

Just trying to think of others

@stolinski
Copy link
Collaborator

Is this something you are working on or just documenting.

@wesbos
Copy link
Collaborator Author

wesbos commented Feb 7, 2024

Just documenting, feel free tot take it on ! Github actions are pain. Worth checking out https://github.com/nektos/act

@wesbos
Copy link
Collaborator Author

wesbos commented Feb 9, 2024

A few more from this morning's scramble:

  1. If a file is committed to the shows folder, it should have a .md extension
  2. Any URLs should resolve - we had some file://Users/.... paths from copy + pasting markdown from VS code
  3. The markdown should parse and be valid - we had an extra --- added to the frontmatter
  4. Youtube links should be full youtube.com links and not youtu.be - not sure if this is a big deal or not, just thought I'd mention it. I feel like linking straight to the video is better than a shortlink - probably some SEO juice there.

@stolinski
Copy link
Collaborator

stolinski commented Feb 12, 2024

Making these into a todo list

  • mp3 url is valid (Fetch with HEAD to ensure file exists)
  • Show number is not overlapping with any existing shows
  • All links in markdown resolve 200 OK
  • Perhaps some sort of spellcheck? (prob a can of worms)
  • Youtube url is a real youtube url (we had them mixed up once)
  • all timestamp links adhere to HH:MM:SS or HH:MM
  • If a file is committed to the shows folder, it should have a .md extension
  • Any URLs should resolve - we had some file://Users/.... paths from copy + pasting markdown from VS code
  • The markdown should parse and be valid - we had an extra --- added to the frontmatter
  • Youtube links should be full youtube.com links and not youtu.be

@stolinski
Copy link
Collaborator

Going to merge the first 2 of these checks and then add on the others.
Here is the test runs #1543

@stolinski
Copy link
Collaborator

Just merged timestamp validation

@wesbos
Copy link
Collaborator Author

wesbos commented Feb 16, 2024

Another thing we should add is making sure the mp3 is only 128kbs bitrate, and/or the file size is not over ~70mb. We had released one that was 128mb and it was causing timeouts on the transcript generation.

We already have ffmpeg wasm package installed, so we could check that way, but honestly checking the filesize would be good enough too.

@wesbos
Copy link
Collaborator Author

wesbos commented Feb 20, 2024

@stolinski This markdown:

* **[32:15](#t=32:15)** Is ChatGPT Plus worth it? I'm trying to avoid death by 1000 subscriptions.
* [Mozilla’s vision for Firefox MV3](https://blog.mozilla.org/addons/2022/11/17/manifest-v3-signing-available-november-21-on-firefox-nightly/

is thinking those numbers in the urls are timestamps

Screenshot 2024-02-20 at 3 36 29 PM

@wesbos
Copy link
Collaborator Author

wesbos commented Mar 1, 2024

@stolinski happened again here Screenshot 2024-03-01 at 11 59 29 AM

@stolinski
Copy link
Collaborator

stolinski commented Mar 1, 2024

Will examine 🤔 need to tweak the regex looking for times.

@wesbos
Copy link
Collaborator Author

wesbos commented Mar 1, 2024

I think the regex might be wrong on purpose for testing? https://github.com/syntaxfm/website/blob/main/scripts/merging-show-validation.js#L31

IMO I'd parse the show notes, and then feed it into linkedom, then you can just document.querySelector([href^="#t="]` to get all the correct links instead of regexing everything

@stolinski
Copy link
Collaborator

stolinski commented Mar 1, 2024 via email

@stolinski
Copy link
Collaborator

stolinski commented Mar 1, 2024 via email

@wesbos
Copy link
Collaborator Author

wesbos commented Mar 1, 2024

Yeah, plus rendering the markdown will also tell us if the markdown will render on the site. Easy way to detect any issues

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

No branches or pull requests

2 participants