-
Notifications
You must be signed in to change notification settings - Fork 0
Update description of config_automation and screenshots #15
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
Conversation
OTTR Check ResultsSummary
🎉 All checks passed!Last Updated: 2025-06-09-16:25:14 |
|
Re-rendered previews from the latest commit:
* note not all html features will be properly displayed in the "quick preview" but it will give you a rough idea. Updated at 2025-06-09 with changes from the latest commit 4ccd9a8 |
|
I don't think the rendering docker image is mentioned there. Note to check if it's mentioned in the customize docker page, or if I need to add a spot for it in the actions page |
oh might be nice to add something brief |
Just checked - looks like we cover this here :) |
|
This looks great, Kate! Thanks for adding this! |
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Co-authored-by: Carrie Wright <23014755+carriewright11@users.noreply.github.com>
Fix spelling
avahoffman
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.
Thanks for adding this @kweav !!
customize-robots.Rmd
Outdated
| spell_error_min: 0 | ||
| ``` | ||
|
|
||
| You'll need to resolve those before being able to merge your pull request. Errors are displayed in much the same way as URL check errors with a summary of how many errors are found. Click on the output comment on your pull request that says "Click here to download detailed error reports". This will download a zip file containing a list of the spelling errors it found. |
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.
Is this strictly true? Isn't it more like "strongly recommended"
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.
I believe you're correct that it's more of a strongly recommend -- someone could set up repo settings such that all checks/workflows have to pass before they can merge, but they can still merge by clicking a bypass button. @carriewright11 do we want to soften the description at all since it's more of a strongly recommended than a requirement for default settings?
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.
Yes we should change to say instead that the check will still show as failing unless they are resolved and we can say that we strongly encourage resolving checks or adjusting the limit before merging to stay on top of spelling errors
|
Good to merge once we settle the last outstanding feedback comment because we've merged the PR with the render fix |
switching screenshot paths from this branch to main
To address issue #13. I've updated some screenshots as well as description throughout the section, not just the highlighted code chunk of the config file.
Will need to update the screenshot URLs to point to main before merging