sysid / sse-starlette

BSD 3-Clause "New" or "Revised" License
505 stars 36 forks source link

Contribution guidelines missing #50

Closed vrslev closed 1 year ago

vrslev commented 1 year ago

Hey there! I have an issue with your project and was willing to try to fix it myself or at least make minimal reproducible example for issue. First, I need to install the project locally to test it out, look at codebase, etc—but there's no guide for that. Packaging system is unclear to me: you have a pipfile and setup.py. Also a makefile that doesn't have install instruction. What is the preferred way to install it for development?

sysid commented 1 year ago

Valid point @vrslev, this is still todo for me. For installation, just install via pipenv and everything should work. I am using direnv for switching venvs, so you also might look at .envrc.

Would you like to tell me the issue you are having?

vrslev commented 1 year ago

Got it. At first, I thought the issue is with this library, but it turned out it was watchfiles. I had FastAPI app with SSE endpoint that yielded files system changes.

paxcodes commented 1 year ago

For anyone else interested, what works for me:

  1. install pipenv: pip install pipenv
  2. install dependencies using pipenv: pipenv install --dev -e .
    • --dev flag will install packages like pytest
    • -e . is to install the sse-starlette package in editable mode (as opposed to having sse-starlette be installed in site-packages). This way, I can modify sse-starlette and when running tests through pytest, it runs code where my changes are applied. --> This is how I understand it anyway.
    • When I pass -e . some files in the project were changed / new files were added; I just discarded those and things should still work as expected: you'd be able to run pytest with your changes to the sse-starlette package.
  3. To run tests, either:
    • pipenv shell and then pytest
    • or pipenv run pytest
sysid commented 1 year ago

@paxcodes I added your documentation to the README. Thanks again.