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

yacht: add exercise #807

Merged
merged 6 commits into from
May 27, 2024
Merged

yacht: add exercise #807

merged 6 commits into from
May 27, 2024

Conversation

SimaDovakin
Copy link
Contributor

@SimaDovakin SimaDovakin commented Apr 22, 2024 •
edited
Loading

Summary

For #48in24. I used the Clojure implementation as a reference.

Checklist

  • If docs where changed run ./bin/configlet generate to ensure all documents are properly generated.
  • CI is green

@github-actions github-actions bot closed this Apr 22, 2024
@verdammelt verdammelt reopened this Apr 22, 2024
@SimaDovakin
Copy link
Contributor Author

@verdammelt Hi! Could we merge it, or should I change something?

verdammelt
verdammelt requested changes May 14, 2024
View reviewed changes
Copy link
Member

@verdammelt verdammelt left a comment

Choose a reason for hiding this comment

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

I think this is a good start... but I would like to discuss some small changes to make it, IMHO, better.

exercises/practice/yacht/.meta/example.lisp Outdated Show resolved Hide resolved
exercises/practice/yacht/.meta/example.lisp Outdated Show resolved Hide resolved
exercises/practice/yacht/.meta/example.lisp Outdated Show resolved Hide resolved
exercises/practice/yacht/.meta/example.lisp Outdated Show resolved Hide resolved
exercises/practice/yacht/.meta/example.lisp Outdated Show resolved Hide resolved
exercises/practice/yacht/.meta/example.lisp Outdated Show resolved Hide resolved
@verdammelt
Copy link
Member

@verdammelt Hi! Could we merge it, or should I change something?

Sorry it took me so long to respond. But see my comments above.

@SimaDovakin
Copy link
Contributor Author

@verdammelt Thank you for review. I will improve my solution.

@exercism exercism deleted a comment from github-actions bot May 14, 2024
@SimaDovakin
Copy link
Contributor Author

@verdammelt Is it ok now?

verdammelt
verdammelt reviewed May 25, 2024
View reviewed changes
Copy link
Member

@verdammelt verdammelt left a comment

Choose a reason for hiding this comment

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

I think this is looking good, but using keywords rather than strings would be better.

exercises/practice/yacht/.meta/example.lisp Outdated Show resolved Hide resolved
@SimaDovakin
Copy link
Contributor Author

@verdammelt Now categories are keywords, and there is a case instead of cond in the main function.

verdammelt
verdammelt approved these changes May 27, 2024
View reviewed changes
Copy link
Member

@verdammelt verdammelt left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time for all the updates.

@verdammelt verdammelt merged commit aa05156 into exercism:main May 27, 2024
3 checks passed
@SimaDovakin
Copy link
Contributor Author

Thank you for all your reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@verdammelt verdammelt verdammelt approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@SimaDovakin @verdammelt