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: adding openapi upload #1116

Merged
merged 73 commits into from
Dec 12, 2024
Merged

Conversation

kanadgupta
Copy link
Member

@kanadgupta kanadgupta commented Dec 10, 2024

🚥 Resolves RM-11384

🧰 Changes

rdme openapi has now been replaced by rdme openapi upload 🚀

highlights:

  • much simpler flag structure
  • more intuitive slug-based identifying system, thanks to API v2
  • better error messages, thanks to API v2

code review notes

  • a lot of the diff is related to setting up boilerplate for interacting with API v2
  • feedback appreciated on the docs — feel free to review documentation/commands/openapi.md or src/commands/openapi/upload.ts
  • feedback appreciated on whether the optional flag should be called --slug or --identifier or something else. went with --slug since it's short and it was easy to write clear explainer docs around
outstanding tasks (archived)
  • command
  • tests
  • optional --action arg? edit: might be a good enhancement further down the line, but going to pass on this for now!
  • tests for useSpecVersion
  • test for pending timeout (there's a nock API for duplicating an interceptor, use that!)
  • finalize command docs
  • set up test project for syncing in CI (i.e., adding back what i removed in d69d1aa)
follow-up enhancements
  • tests for readmeAPIv2Fetch
  • optional --action arg for use in CI to prevent accidental overwrites
  • optional --timeout arg to allow user to configure timeout
  • GHA onboarding support

🧬 QA & Testing

this can only be tested locally for now:

  1. in your local dev server, load up a project that uses readme refactored
  2. set up an API key for said project
  3. change the value on this line to be http://api.readme.local:3000/v2
  4. check out this branch and run the following:
# setup
npm ci && npm run build

# try running any or all of the following:
bin/dev.js openapi upload # try logging in
bin/dev.js openapi upload --key <key> # try selecting a file
bin/dev.js openapi upload --key <key> __tests__/__fixtures__/petstore-simple-weird-version.json # without a version arg
bin/dev.js openapi upload --key <key> __tests__/__fixtures__/petstore-simple-weird-version.json --version=1.0 # with a version arg
bin/dev.js openapi upload --key <key> __tests__/__fixtures__/petstore-simple-weird-version.json --useSpecVersion # use the version defined in the spec

@kanadgupta kanadgupta changed the base branch from next to kanad-2024-12-06/remove-openapi December 10, 2024 01:16
kanadgupta added a commit that referenced this pull request Dec 11, 2024
## 🧰 Changes

Removes the `openapi` command in its current form. Will be replaced by
`openapi upload` (see #1116!)

Will be merging this PR into #1113
to collect all breaking changes as part of this PR!

## 🧬 QA & Testing

Provide as much information as you can on how to test what you've done.

## ⚠️ breaking changes

BREAKING CHANGE: `rdme openapi` has been removed. Please use `rdme
openapi upload` instead. Read more in [our migration
guide](https://github.com/readmeio/rdme/tree/v10/documentation/migration-guide.md).
Base automatically changed from kanad-2024-12-06/remove-openapi to v10-release December 11, 2024 18:39
kanadgupta added a commit that referenced this pull request Dec 12, 2024
## 🧰 Changes

cherry picks a few refactors from
#1116 to minimize that diff and
bring these changes to the v9 channel

## 🧬 QA & Testing

do tests still pass?
@kanadgupta kanadgupta added enhancement New feature or request command:openapi Issues pertaining to the `openapi`, `validate`, `reduce`, or `swagger` commands documentation Improvements or additions to documentation labels Dec 12, 2024
@kanadgupta kanadgupta marked this pull request as ready for review December 12, 2024 00:47
__tests__/helpers/get-api-mock.ts Show resolved Hide resolved
__tests__/helpers/get-api-mock.ts Show resolved Hide resolved
src/commands/openapi/upload.ts Show resolved Hide resolved
src/commands/openapi/upload.ts Show resolved Hide resolved
src/commands/openapi/upload.ts Show resolved Hide resolved
src/lib/apiError.ts Outdated Show resolved Hide resolved
src/lib/readmeAPIFetch.ts Show resolved Hide resolved

headers.set('x-readme-source', source);

if (fileOpts.filePath && fileOpts.fileType === 'url') {
Copy link
Member

Choose a reason for hiding this comment

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

This may get run twice if isGHA() is true and we have a file there.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah yikes yeah this would overwrite that. we don't do anything with this header at the moment but i'll look into why i wrote it this way, this is just holdover from the API v1 code

@kanadgupta kanadgupta merged commit 8a33ed4 into v10-release Dec 12, 2024
8 checks passed
@kanadgupta kanadgupta deleted the kanad-2024-12-06/oas-v2-RM-11384 branch December 12, 2024 22:14
kanadgupta added a commit that referenced this pull request Dec 12, 2024
## 🧰 Changes

this PR aggregates all of the PRs going out as part of the v10 release
(i.e, the second section of PRs below). all PRs should be reviewed prior
to being merged into this branch.

### outstanding tasks

#### needs to go out _before_ v10 is released (i.e., in the v9 release
channel)
- [x] #1082


#### needs to go out as part of v10 release
- [x] `openapi upload`
  - [x] #1111
  - [x] #1116
- [x] #1107
- [x] #1104
- [x] #1108

#### merge into `v9` branch once `v10` release is successfully released
- [ ] #1121

#### double-check these things before merging

- [x] swap out any links to the `v9` docs (e.g., `/tree/v9`) with `v10`
as needed
(b19416d)
- [x] make sure all API v1 requests in `v10` will work
- [x] make sure v10 migration guide reflects the final design decisions
around `openapi upload`

## ⚠️ Breaking Changes

<sub>listing all of the breaking changes 1 by 1 below so they get picked
up by semantic release...</sub>

BREAKING CHANGE: `categories`, `custompages`, `docs` and `versions` have
now been removed. Please use a bidirectional syncing workflow instead.
Read more in [our migration
guide](https://github.com/readmeio/rdme/tree/v10/documentation/migration-guide.md).

BREAKING CHANGE: `rdme openapi` has been replaced by `rdme openapi
upload`. Read more in [our migration
guide](https://github.com/readmeio/rdme/tree/v10/documentation/migration-guide.md).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command:openapi Issues pertaining to the `openapi`, `validate`, `reduce`, or `swagger` commands documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants