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.

Sign up for GitHub

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

Jump to bottom

Introduce testing infrastructure #37

Merged
merged 14 commits into from
Mar 28, 2023
Merged

Introduce testing infrastructure #37

merged 14 commits into from
Mar 28, 2023

Conversation

rgwohlbold
Copy link
Member

Closes #35

@rgwohlbold rgwohlbold marked this pull request as ready for review March 23, 2023 16:13
@rgwohlbold rgwohlbold changed the title Draft: First working testing First working testing Mar 23, 2023
@rgwohlbold rgwohlbold changed the title First working testing Introduce testing infrastructure Mar 28, 2023
@rgwohlbold rgwohlbold requested review from dasGoogle, linaScience and Sohn123 and removed request for dasGoogle and linaScience March 28, 2023 09:19
Sohn123
Sohn123 approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@Sohn123 Sohn123 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think removing the magic numbers makes the code more readable, and splitting the config file reading and autoconfiguring the sockets is good. :shipit:

@Sohn123 Sohn123 merged commit 38ca245 into main Mar 28, 2023
@Sohn123 Sohn123 deleted the 35-add-testing-infrastructure branch March 28, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sohn123 Sohn123 Sohn123 approved these changes

@dasGoogle dasGoogle Awaiting requested review from dasGoogle

@linaScience linaScience Awaiting requested review from linaScience

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add testing infrastructure
2 participants
@rgwohlbold @Sohn123