exercism / scala

Exercism exercises in Scala.
https://exercism.org/tracks/scala
MIT License
123 stars 131 forks source link

Add Yacht exercise to the Scala track #789

Closed wangxiaoxuan273 closed 1 year ago

wangxiaoxuan273 commented 1 year ago

Per my discussion thread on the exercism forum.

github-actions[bot] commented 1 year ago

Hello. Thanks for opening a PR on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed.

That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there.


Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.

wangxiaoxuan273 commented 1 year ago

@ErikSchierboom Could you please take a look at this pull request? I did not build, run or locally tested anything, since I did not find any guide about how to do any of these, so this PR may need further change. At least I know the tests.toml file is missing.

I mainly copied the stuff from the F# Yacht problem and translated everything into Scala 2.

ErikSchierboom commented 1 year ago

configlet fails with:

/home/runner/work/scala/scala/exercises/practice contains a directory named yacht, which is not a slug in the array of practice exercises. Please add the exercise to that array. If the exercise is not ready to be shown on the website, please set its status value to "wip":

You should add an entry for this exercise to the exercises.practice array in the config.json file. Note that the uuid must be uniquely generated (you can do this via bin/fetch-configlet && bin/configlet uuid.

I'd also recommend update the tests, metadata and docs with the latest version: bin/fetch-configlet && bin/configlet sync --update --yes --tests include --docs --metadata.

All in all this is a great PR!

wangxiaoxuan273 commented 1 year ago

configlet fails with:

/home/runner/work/scala/scala/exercises/practice contains a directory named yacht, which is not a slug in the array of practice exercises. Please add the exercise to that array. If the exercise is not ready to be shown on the website, please set its status value to "wip":

You should add an entry for this exercise to the exercises.practice array in the config.json file. Note that the uuid must be uniquely generated (you can do this via bin/fetch-configlet && bin/configlet uuid.

I'd also recommend update the tests, metadata and docs with the latest version: bin/fetch-configlet && bin/configlet sync --update --yes --tests include --docs --metadata.

All in all this is a great PR!

Done these steps! When running bin/fetch-configlet && bin/configlet sync --update --yes --tests include --docs --metadata, the program looked for config.json in bin and could not find it. I copied the file from the root directory to bin to let the program run, and did not commit it after the program had completed.

wangxiaoxuan273 commented 1 year ago

@ErikSchierboom I have updated this pr according to your comments. Please take a look again, thanks.

ErikSchierboom commented 1 year ago

Done these steps! When running bin/fetch-configlet && bin/configlet sync --update --yes --tests include --docs --metadata, the program looked for config.json in bin and could not find it. I copied the file from the root directory to bin to let the program run, and did not commit it after the program had completed.

Are you sure? configlet expects config.json in the root. Did you run it from the root? I also forgot that --exercise yacht should be added to the command. Could you redo the last commit by running the previous command but with --exercise yacht added?

wangxiaoxuan273 commented 1 year ago

Done these steps! When running bin/fetch-configlet && bin/configlet sync --update --yes --tests include --docs --metadata, the program looked for config.json in bin and could not find it. I copied the file from the root directory to bin to let the program run, and did not commit it after the program had completed.

Are you sure? configlet expects config.json in the root. Did you run it from the root? I also forgot that --exercise yacht should be added to the command. Could you redo the last commit by running the previous command but with --exercise yacht added?

I did not run it in the root, I ran it inside the bin. It looks like the program looked for the config in the current directory.

Okay, I'll redo the last commit.

wangxiaoxuan273 commented 1 year ago

I have updated this pr according to your latest comments. Please take a look. @ErikSchierboom

wangxiaoxuan273 commented 1 year ago

The CI tests failed. I'll work on fixing my errors.

wangxiaoxuan273 commented 1 year ago

I have changed my implementation and run the tests locally and they passes. Can you start the workflow again to see if the CI tests pass this time? @ErikSchierboom

wangxiaoxuan273 commented 1 year ago

I fixed my error of the wrong class name. Could you please start the CI test again? Thanks. @ErikSchierboom

ErikSchierboom commented 1 year ago

Thanks!