Skip to content
This repository was archived by the owner on Aug 19, 2025. It is now read-only.

Conversation

@hramezani
Copy link
Contributor

@hramezani hramezani commented Mar 2, 2020

Fixes #6.

I used GitHub action services instead of docker-compose.

@rafalp
Copy link
Contributor

rafalp commented Mar 15, 2021

Could you please implement Tom's feedback to your PR so it follows conventions from other encode projects and also implements code coverage? Thanks!

@hramezani
Copy link
Contributor Author

Sure! I will do them and will let you know.

@hramezani
Copy link
Contributor Author

@rafalp I made it like other encode project(httpx) as Tom mentioned.
I just disabled the docs build because we don't have any doc for this project.

Please take a look and let me if there is anything else to do.

scripts/docs Outdated
@@ -0,0 +1,10 @@
#!/bin/sh -e
Copy link
Contributor

Choose a reason for hiding this comment

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

This file seems useless considered we don't have any docs for the lib.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed this script

import typing
from typing import Any

from broadcaster._base import Event
Copy link
Contributor

Choose a reason for hiding this comment

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

This import should be relative.

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!

from .base import BroadcastBackend
from .._base import Event

from broadcaster._backends.base import BroadcastBackend
Copy link
Contributor

Choose a reason for hiding this comment

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

Those imports should be relative.

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!

from .base import BroadcastBackend
from .._base import Event

from broadcaster._backends.base import BroadcastBackend
Copy link
Contributor

Choose a reason for hiding this comment

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

Those imports should be relative.

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!


import asyncio_redis

from broadcaster._backends.base import BroadcastBackend
Copy link
Contributor

Choose a reason for hiding this comment

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

Those imports should be relative,

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!

@hramezani
Copy link
Contributor Author

@rafalp I addressed your comments.

@rafalp rafalp merged commit 435c35e into encode:master Mar 19, 2021
@rafalp
Copy link
Contributor

rafalp commented Mar 19, 2021

Nice work, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setup CI

2 participants