-
Notifications
You must be signed in to change notification settings - Fork 4
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
Chore: housekeeping #3
base: main
Are you sure you want to change the base?
Conversation
Looks great so far, but I get you are still working on it before we should all review it properly, right? |
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.
nice clean-up!
Yes, I still have some things I want to do. Should I split the PR into at least two? Maybe one that only has the Redis changes? |
Good idea, let's start to have that merged in independently |
92abe92
to
89a57df
Compare
89a57df
to
17d99cc
Compare
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
…dev outside a container Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
f0a6058
to
610abe3
Compare
Signed-off-by: André Carvalho <[email protected]>
610abe3
to
3fa5c9b
Compare
|
||
- name: Test | ||
# Wait for RabbitMQ to start. | ||
run: sleep 10 && make test |
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.
I don't like this; it would be nice for the tests to wait for RabbitMQ to actually be ready. Also, for some reason the tests didn't do anything for 10 minutes on 3 runs, so maybe this should be disabled?
Suggestions welcome.
@RedRoserade do you understand why some tests take > 1h? Maybe the matrix strategy has some colliding host? |
I don't know. I saw builds yesterday hanging with no output on the test stage, and there's currently builds going on for 4 hours as of this comment? That's not normal at all, maybe the test and docker part should be turned off for now. |
I would propose to turn off the matrix, and see if testing only with |
Signed-off-by: André Carvalho <[email protected]>
Signed-off-by: André Carvalho <[email protected]>
This does a few changes, and should probably be split into multiple PRs for digestibility.
aioredis
toredis-py
, because the former is deprecated and moved into the main packageaioredis
)isort
for sorting imports. Because of the large diff, an "ignore revs" file has been createddocker-compose.yaml
file just for Redis and RabbitMQMakefile
with common commands to format code, lint, and testStill want to implement: