Skip to content

Conversation

@vincerubinetti
Copy link
Contributor

@vincerubinetti vincerubinetti commented Feb 18, 2025

Related: CU-DBMI/redirects-website#2

This PR is in response to a user encountering workflow failures and not understanding why, because they had neglected to include a to field in a few of their entries. Currently, the logging prints the "entry number" (the index in the array/list), but that's not super helpful when looking at raw yaml because each entry will take up at least two lines (can have empty lines between entries, comments, etc). Printing a line number would be most helpful, but is not very simple in the yaml parsing library I'm using, or in any other library I saw. So instead I'm printing just the stringified entry, or another field value when appropriate. This should help prevent user errors in the future. Now it will look like:

"to" field missing
  redirects.yaml entry #1 {"from":"chatroom"}
"from" field missing
  redirects.yaml entry #2 {"to":"https://docs.google.com/document/d/12345abcdef"}

Instead of:

redirects.yaml entry 131 "to" field invalid
  • install chalk for the scripts, and upgrade to latest versions of packages
  • make "check" script name more descriptive
  • rewrite/rework readme to be more helpful, more consistent, more accurate, and to use a inlined mermaid diagram instead of an image
  • rework "check" script
    • move status codes to top, and to account for new lists structures (see below)
    • tweak error logging
  • rework "core" script
    • tweak error logging
    • rework check logic to be more robust
    • add better colored logging
    • add common trace util func
  • rework "encode" script
    • use new util funcs

@vincerubinetti
Copy link
Contributor Author

Also note that the "check broken" workflow is failing, and this is to be expected because of my fake https://zoom.us/j/12345abcdef example links. I suppose I could replace these with real links, but then I'd have to make sure they stay valid for the lifetime of this project. Or... we could just say that we should keep this as failing, so that when a user copies these templates, they get an error if they forget to remove the example links.

Copy link

@falquaddoomi falquaddoomi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all great changes; I like how you've rewritten the docs for a lay audience, and the changes to what the checker scripts report are way more readable and actionable. Nice work!

Regarding the link-checker workflow failing by default due to the dummy links, while it's unusual to ship failing tests I think, like you said, it's good motivation for them to replace the links with real ones.

@vincerubinetti
Copy link
Contributor Author

Fixed some last minute typos and forgot to force-enable colors on github actions:
https://github.com/chalk/supports-color/blob/ae809ecabd5965d0685e7fc121efe98c47ad8724/index.js#L38

@vincerubinetti vincerubinetti merged commit a97254f into main Feb 19, 2025
1 of 2 checks passed
@vincerubinetti vincerubinetti deleted the refresh branch February 19, 2025 17:54
@jmcmurry
Copy link
Collaborator

thanks!

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.

4 participants