-
Notifications
You must be signed in to change notification settings - Fork 24
README improvements #622
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
base: main
Are you sure you want to change the base?
README improvements #622
Conversation
|
Claude finished @harr1424's task in 2m 1s —— View job Code Review Complete
SummaryOverall Assessment: ✅ APPROVE The documentation improvements are accurate and helpful. The PR correctly documents:
The initial error suggesting manual file copying was correctly fixed following dani-garcia's feedback. |
|
Great job! No new security vulnerabilities introduced in this pull request |
🔍 SDK Breaking Change Detection ResultsSDK Version:
Breaking change detection completed. View SDK workflow |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #622 +/- ##
=======================================
Coverage 78.54% 78.54%
=======================================
Files 283 283
Lines 29251 29251
=======================================
Hits 22975 22975
Misses 6276 6276 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
README.md
Outdated
| The first step is to generate the swagger documents from the root of the server repository. | ||
| The first step is to generate the swagger documents from the root of the | ||
| [server repository](https://github.com/bitwarden/server), and then copy the resulting `api.json` and | ||
| `api.public.json` to the root of the SDK repository. |
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 files that we need are api.json and identity.json, and there's no need to copy them, the script reads them from the server folder directly as long as the server and sdk folders are at the same directory:
sdk-internal/support/build-api.sh
Lines 10 to 11 in 0a9599e
| cp ../server/api.json ./artifacts/api.json | |
| cp ../server/identity.json ./artifacts/identity.json |
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.
We should mention somewhere that you should only use the workflow and never merge manual binding changes in PRs.
https://github.com/bitwarden/sdk-internal/actions/workflows/update-api-bindings.yml

📔 Objective
Improve
READMEto provide greater detail on the process of generating API bindings.⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes