-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat(docs): adds ability to delete docs from ReadMe if they're no longer in local folder #581
feat(docs): adds ability to delete docs from ReadMe if they're no longer in local folder #581
Conversation
…an only available spec file without any prompts
This reverts commit 95a8148.
…Me documents that don't exist on the local folder
@@ -9,6 +9,8 @@ import APIError from './apiError'; | |||
import isGHA from './isGitHub'; | |||
import { debug } from './logger'; | |||
|
|||
const SUCCESS_NO_CONTENT = 204; |
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.
The DELETE /api/v1/docs/{slug}
endpoint returns 204 with an empty body if the deletion was successful.
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.
one minor comment but overall this seems good to me
src/lib/getDocs.ts
Outdated
.then(Promise.all.bind(Promise)) | ||
.then(args => [].concat(...args)); |
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.
These two lines are difficult to follow, any way they can be simplified?
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.
Done (I think ;) )
…add-delete-missing-option
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 so much for adding this. LGTM but letting @kanadgupta have final sign off before it's merged.
With pleasure, that's what OpenSource is all about :) Is there an expected timeline for the last two changes to be published to NPM? |
We're prepping for a big v8 release, so it won't be for at least another week or two! |
Week or two is great :) |
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.
A few small comments below but this is looking good! Thanks @shaiarmis 🙂
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 your patience and work on this @shaiarmis! LGTM 🥳
🧰 Changes
The
rdme docs
command syncs the documents between a local folder and a ReadMe project.It works really well for new file in the folder, or for editing existing ones, but it does't work for files that were deleted and are no longer needed.
With the new
--deleteMissing
option (off by default) this is now possible.🧬 QA & Testing
--deleteMissing
)