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

SM-1098: Update Python Wrapper README Instructions #592

Merged
merged 6 commits into from
Feb 13, 2024
Merged

Conversation

coltonhurst
Copy link
Member

Type of change

- [ ] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [x] Other

Objective

Update the Python Wrapper README, now that we are using maturin.

@coltonhurst coltonhurst marked this pull request as ready for review February 7, 2024 21:35
@coltonhurst coltonhurst self-assigned this Feb 7, 2024
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8ea2f79) 59.43% compared to head (a898929) 59.43%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #592   +/-   ##
=======================================
  Coverage   59.43%   59.43%           
=======================================
  Files         171      171           
  Lines       10320    10320           
=======================================
  Hits         6134     6134           
  Misses       4186     4186           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bitwarden-bot
Copy link

bitwarden-bot commented Feb 8, 2024

Logo
Checkmarx One – Scan Summary & Detailsd946f2ee-9030-4137-bd2c-f94df383f46b

No New Or Fixed Issues Found

@@ -31,5 +32,8 @@ pip install bitwarden-sdk
Set the `ORGANIZATION_ID` and `ACCESS_TOKEN` environment variables to your organization ID and access token, respectively.

```bash
source .venv/bin/activate
Copy link
Member

Choose a reason for hiding this comment

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

This is under run and we do not know if they use a venv or some other approach.

Copy link
Member Author

@coltonhurst coltonhurst Feb 8, 2024

Choose a reason for hiding this comment

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

Moved this to a Using Virtual Environments section, away from the Run section. If anything, it will hopefully help those who are less familiar with Python get started.

d2b769a

@@ -12,6 +12,7 @@ From the root of the repository:
npm run schemas # generate schemas.py

cd languages/python/
python3 -m venv .venv
Copy link
Member

Choose a reason for hiding this comment

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

Should we move this under virtual env? This command doesn't mount the venv either.

Copy link
Member Author

@coltonhurst coltonhurst Feb 9, 2024

Choose a reason for hiding this comment

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

Moved everything to the separate section for running & building with virtual environments here:

582b09f

@coltonhurst coltonhurst requested a review from Hinton February 9, 2024 20:45
@coltonhurst coltonhurst merged commit 16f4cc5 into main Feb 13, 2024
58 of 59 checks passed
@coltonhurst coltonhurst deleted the sm/sm-1098 branch February 13, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants