-
Notifications
You must be signed in to change notification settings - Fork 626
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(readme): API documentation overhaul #716
Conversation
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.
great PR, the rewording is very good!
I've created a commit on my fork to put all the suggestions I had (seemed easier than writing down everything in a review). all changes are obviously open to discussion!
you can view the diff here: rharshit82/node-cron@fix/readme-overhaul...sheerlox:node-cron:docs/readme/api-documentation-overhaul
if you'd like to integrate my commit in your branch, you can pick the patch file and apply it in your branch:
curl https://github.com/sheerlox/node-cron/commit/1d22cf136b5596d4ae2e43fed35d427b3c78579b.patch | git am
you can also just apply the changes (without creating the commit) by piping the patch into git apply
instead of git am
.
Thanks @sheerlox I have added your commit. I was actually going to fix table of contents and refactor the readme more but your commit as done just that. Marking this PR ready for review ! |
docs(readme): minor capitalization added
6f02030
to
3647be4
Compare
lgtm, I'll let @intcreator take a look as well so he can make changes as he sees fit before merging. Note: I've added two badges that are not yet functional: the CodeQL one will start working when #715 is merged, and the Discord badge needs a tweak in the Discord server settings (enable widget) to display the number of online members correctly. |
Sure added it. Didnt add it before since I thought its only for a code change that required a doc change. |
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 have a few questions/requested changes, but overall I like it! I especially like...
- addition of Discord badge (I enabled the widget so it shows a preview of online people too)
- table of contents
(and I was just joking about the checklist item haha)
Have made the requested changes. Feel free to make commits in this PR if needed @sheerlox |
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 just added the Renovate badge. I think that's everything, looks good to me
since we're planning on merging #751 today, I skipped the CI so we release them together under |
## [3.1.5](v3.1.4...v3.1.5) (2023-10-26) ### 🐛 Bug Fixes * detect multiple zeros as an invalid step ([#743](#743)) [skip ci] ([b0bf677](b0bf677)) * re-add runOnce property to CronJob ([#751](#751)) ([a61d8c9](a61d8c9)) ### 📚 Documentation * **readme:** API documentation overhaul ([#716](#716)) [skip ci] ([23fb0a3](23fb0a3)) ### ⚙️ Continuous Integrations * **action:** update actions/setup-node action to v4 ([#749](#749)) ([ef850f3](ef850f3)) ### ♻️ Chores * **deps:** update dependency [@commitlint](https://github.com/commitlint)/cli to v18 ([#747](#747)) ([5ff1cf8](5ff1cf8)) * **deps:** update dependency sinon to v17 ([#748](#748)) ([9d61ff9](9d61ff9)) * **deps:** update linters ([7bdc726](7bdc726)) * improve ossf scorecard's score ([#715](#715)) [skip ci] ([1284df4](1284df4))
🎉 This PR is included in version 3.1.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Refactored the Readme file into a more readable and standard readme
Related Issue
Closes #441.
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
!
after the type/scope in the title (see the Conventional Commits standard).