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

Add DataSchema, docs, tests, config improvements #1

Merged
merged 3 commits into from
Dec 16, 2024
Merged

Conversation

luk-pau-es
Copy link
Collaborator

Add DataSchema for mapping XML to Elixir structures
Add tests
Configuration improvements
Add new message handlers
Add message Mapper
Add docs and ensure docs generated with mix docs (ExDoc) are correct
Add License
Add proper readme
Setup GH Actions for Elixir
Add .tool-versions

@luk-pau-es luk-pau-es self-assigned this Nov 29, 2024
@luk-pau-es
Copy link
Collaborator Author

I have put an Apache license in this repo, should I change it to some other?

@luk-pau-es
Copy link
Collaborator Author

Also in regards to testing, I am thinking about expanding the scope of it, because as of now, tests are based on one or 2 different messages, but what can be done is to gather subset of different messages and then doing something similar to property based testing, but I would need to come up with some way of handling comparison between mapping output and expected output for adding new cases and so on

@luk-pau-es luk-pau-es requested a review from erszcz December 2, 2024 09:05
Copy link
Member

@erszcz erszcz left a comment

Choose a reason for hiding this comment

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

Great work, @luk-pau-es 🚀
I've left a few comments - some just as notes for the future, but some would be good to address still in this PR. This thing looks good!

lib/uof_feed.ex Outdated Show resolved Hide resolved
lib/uof_feed/amqp/config.ex Outdated Show resolved Hide resolved
lib/uof_feed/amqp/config.ex Outdated Show resolved Hide resolved
lib/uof_feed/amqp/config.ex Outdated Show resolved Hide resolved
lib/uof_feed/amqp/client.ex Show resolved Hide resolved
lib/uof_feed/mapper.ex Outdated Show resolved Hide resolved
lib/uof_feed/messages/bet_settlement.ex Outdated Show resolved Hide resolved
lib/uof_feed/messages/fixture_change.ex Show resolved Hide resolved
lib/uof_feed/messages/market.ex Show resolved Hide resolved
lib/uof_feed/messages/bet_stop.ex Outdated Show resolved Hide resolved
@erszcz
Copy link
Member

erszcz commented Dec 16, 2024

Looks good! 🎉

@luk-pau-es luk-pau-es merged commit d0be3cd into main Dec 16, 2024
1 check passed
@luk-pau-es luk-pau-es deleted the add-data-schema branch December 16, 2024 11:02
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.

2 participants