Skip to content
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

Merged
merged 30 commits into from
Sep 12, 2022

Conversation

shaiarmis
Copy link
Contributor

🧰 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

  1. Sync some files from a folder with your project
  2. Delete one of the files from the folder
  3. Sync again (with --deleteMissing)
  4. The file should be delete from ReadMe

@@ -9,6 +9,8 @@ import APIError from './apiError';
import isGHA from './isGitHub';
import { debug } from './logger';

const SUCCESS_NO_CONTENT = 204;
Copy link
Contributor Author

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.

@kanadgupta kanadgupta added enhancement New feature or request command:docs Issues pertaining to the `docs`, `changelogs`, or `custompages` commands labels Aug 24, 2022
Copy link
Member

@erunion erunion left a 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/fetch.ts Show resolved Hide resolved
@erunion erunion requested a review from kanadgupta August 24, 2022 22:17
src/lib/getDocs.ts Outdated Show resolved Hide resolved
src/lib/deleteDoc.ts Show resolved Hide resolved
src/cmds/docs/index.ts Show resolved Hide resolved
src/lib/getSlug.ts Outdated Show resolved Hide resolved
Comment on lines 36 to 37
.then(Promise.all.bind(Promise))
.then(args => [].concat(...args));
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (I think ;) )

src/cmds/docs/index.ts Outdated Show resolved Hide resolved
src/lib/getDocs.ts Outdated Show resolved Hide resolved
__tests__/cmds/docs/index.test.ts Outdated Show resolved Hide resolved
@kanadgupta kanadgupta self-requested a review August 29, 2022 20:14
@shaiarmis shaiarmis requested review from erunion and removed request for kanadgupta August 30, 2022 07:07
README.md Outdated Show resolved Hide resolved
Copy link
Member

@erunion erunion left a 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.

@shaiarmis
Copy link
Contributor Author

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?

@kanadgupta
Copy link
Member

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!

@shaiarmis
Copy link
Contributor Author

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 :)

@shaiarmis shaiarmis requested a review from kanadgupta September 5, 2022 07:14
Copy link
Member

@kanadgupta kanadgupta left a 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 🙂

__tests__/cmds/docs/index.test.ts Outdated Show resolved Hide resolved
__tests__/cmds/docs/index.test.ts Show resolved Hide resolved
src/lib/deleteDoc.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/lib/prompts.ts Outdated Show resolved Hide resolved
@kanadgupta kanadgupta changed the title Add the ability to delete documents from ReadMe if they're no longer in the folder feat(docs): adds ability to delete docs from ReadMe if they're no longer in local folder Sep 12, 2022
Copy link
Member

@kanadgupta kanadgupta left a 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 🥳

@kanadgupta kanadgupta merged commit 939bce6 into readmeio:main Sep 12, 2022
@shaiarmis shaiarmis deleted the add-delete-missing-option branch September 13, 2022 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command:docs Issues pertaining to the `docs`, `changelogs`, or `custompages` commands enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants