Skip to content

Conversation

@ruidazeng
Copy link
Collaborator

Can we set up a GitHub workflow for this too?

@ruidazeng ruidazeng requested a review from samdevo December 29, 2024 21:21
@ruidazeng
Copy link
Collaborator Author

ruidazeng commented Dec 29, 2024

I don't have too much experience with continuous integration (CI) workflow, but I think we could create a python-app.yml in actions for Python applications.

@ruidazeng
Copy link
Collaborator Author

Never mind, I figured it out. #12

@ruidazeng ruidazeng marked this pull request as draft December 29, 2024 22:39
@ruidazeng
Copy link
Collaborator Author

Need to pass some checks:

Python application / build (push) Failing after 8s
Pylint / build (3.8) (push) Successful in 14s
Pylint / build (3.9) (push) Successful in 13s
Pylint / build (3.10) (push)

@ruidazeng
Copy link
Collaborator Author

Could you approve #12 first? The checks are not passing so I have to refactor my codes a bit more.

@ruidazeng
Copy link
Collaborator Author

@samdevo Continuous integration completed, all code should work with linter and pytest

@ruidazeng ruidazeng marked this pull request as ready for review January 2, 2025 19:23
Copy link
Owner

@samdevo samdevo left a comment

Choose a reason for hiding this comment

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

Nice! Some minor comments

pool_prices.append((pool.rpc_data, price))
print(f"Processed pool {pool.rpc_data} with price {price}")
except ValueError:
print(f"Invalid pool_price '{pool.pool_price}' in pool {pool.rpc_data}; skipping.")
Copy link
Owner

Choose a reason for hiding this comment

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

Where would the ValueError come from?

Copy link
Owner

Choose a reason for hiding this comment

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

pytest files should start with test_* or end with *_test I believe


# Safely handle result
if not isinstance(result, (tuple, list)) or len(result) != 5:
pytest.fail("Invalid result format: expected 5-element sequence")
Copy link
Owner

Choose a reason for hiding this comment

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

why not just use assert?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@samdevo
Copy link
Owner

samdevo commented Jan 13, 2025

And @ruidazeng re your question abt workflows. I think the codebase is too small right now to need workflows for testing. Maybe in the future. But if you want to add and think it's worthwhile fell free!

@ruidazeng
Copy link
Collaborator Author

And @ruidazeng re your question abt workflows. I think the codebase is too small right now to need workflows for testing. Maybe in the future. But if you want to add and think it's worthwhile fell free!

merged with passed tests!

@ruidazeng ruidazeng merged commit d6c4a85 into main Jan 14, 2025
1 check passed
@ruidazeng ruidazeng deleted the arbitrage branch January 14, 2025 13:58
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.

3 participants