-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
docs: Add sponsors to README #36
Conversation
tools/update-readme.js
Outdated
// Main | ||
//----------------------------------------------------------------------------- | ||
|
||
(async () => { |
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 IIFE needed?
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.
Not really, it just survived through multiple copy-pastes from the original code in eslint/eslint, which is in a cjs module so top-level await was not available. I removed it now.
const SPONSORS_URL = | ||
"https://raw.githubusercontent.com/eslint/eslint.org/main/includes/sponsors.md"; | ||
|
||
const README_FILE_PATH = "./README.md"; |
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.
const README_FILE_PATH = "./README.md"; | |
const README_FILE_PATH = "./README.md"; |
Suggestion: Can we use an absolute path here? If we're testing locally from the "tools" directory and run this file using a relative path, it may throw an error. An absolute path would provide consistency in all cases, ensuring the file is accessed correctly regardless of the current working directory.
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.
since we are not going to run this manually so using a relative path is also okay.
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.
LGTM
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.
LGTM, thanks!
Prerequisites checklist
What is the purpose of this pull request?
Adds sponsors to README.md.
What changes did you make? (Give an overview)
Added a script and a workflow that automatically updates sponsors every day.
Related Issues
Is there anything you'd like reviewers to focus on?