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

update runtests.jl #788

Merged
merged 1 commit into from
Sep 18, 2024
Merged

update runtests.jl #788

merged 1 commit into from
Sep 18, 2024

Conversation

depial
Copy link
Contributor

@depial depial commented Sep 18, 2024

I finally got around to completing knapsack and I thought it could benefit from the use of a NamedTuple instead of the struct. I simply modified swapped out the struct and modified the tests to take a tuple. The example.jl remains untouched and still passes, so should all current solutions on the website (three users in total have completed as of writing this).

@depial depial requested a review from colinleach September 18, 2024 20:51
@depial
Copy link
Contributor Author

depial commented Sep 18, 2024

@colinleach, maybe this might be a good time to try putting [no important files changed] in the commit body? There won't be a huge issue if we don't because there are only three solutions so far.

Copy link
Contributor

@colinleach colinleach left a comment

Choose a reason for hiding this comment

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

Sure: this is a rerun of the discussion we had around grade-school, and I support the change.

@colinleach
Copy link
Contributor

this might be a good time to try putting [no important files changed] in the commit body

I'm happy either way, so it's your choice. I think you can merge when you're ready, now I've approved, but let me know if I have to do it. Many aspect of GitHub are still a bit mysterious to me.

@depial depial merged commit ed6947c into exercism:main Sep 18, 2024
12 checks passed
@depial
Copy link
Contributor Author

depial commented Sep 18, 2024

I didn't know I was going to be able to, but I've merged it. I had been merging yours after reviewing since I thought it was necessary for the reviewer to do so, but maybe I can just leave those for you to merge at your convenience from now on.

@colinleach
Copy link
Contributor

It's all a bit confusing. Maybe a trial-and-error approach will get us through the early months of this.

@depial
Copy link
Contributor Author

depial commented Sep 18, 2024

Let's hope so! :)

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