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

Starts docs and exercises, implement verify-exercises #3

Merged
merged 14 commits into from
Aug 21, 2024

Conversation

ageron
Copy link
Contributor

@ageron ageron commented Aug 19, 2024

Add a first version of each documentation file.
Also add the first two practice exercises, hello-world and leap, including example code and tests.
Lastly, implement verify-exercises (all tests pass).
configlet lint and configlet sync are happy. 👍

config.json Outdated Show resolved Hide resolved
Copy link
Contributor

@Anton-4 Anton-4 left a comment

Choose a reason for hiding this comment

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

Thanks @ageron, this looks great! I just left a couple minor comments.

docs/SNIPPET.txt Show resolved Hide resolved
config.json Outdated Show resolved Hide resolved
exercises/practice/leap/Leap.roc Outdated Show resolved Hide resolved
highlightjs does not support Roc, haskell should work ok
docs/ABOUT.md Outdated Show resolved Hide resolved
exercises/practice/leap/.meta/config.json Outdated Show resolved Hide resolved
exercises/shared/.docs/tests.md Outdated Show resolved Hide resolved
docs/LEARNING.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Show resolved Hide resolved
docs/ABOUT.md Show resolved Hide resolved
@Anton-4
Copy link
Contributor

Anton-4 commented Aug 19, 2024

I addressed all comments, is this good to merge @ErikSchierboom?

Clarify About.md

Co-authored-by: András B Nagy <20251272+BNAndras@users.noreply.github.com>
Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Just a couple of comments.

docs/LEARNING.md Outdated Show resolved Hide resolved
docs/SNIPPET.txt Outdated Show resolved Hide resolved
exercises/practice/leap/Leap.roc Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
ageron and others added 2 commits August 21, 2024 09:14
Co-authored-by: András B Nagy <20251272+BNAndras@users.noreply.github.com>
@ageron
Copy link
Contributor Author

ageron commented Aug 20, 2024

Thank you all for reviewing this PR. 👍 I think I've addressed all the points you raised.

@ageron
Copy link
Contributor Author

ageron commented Aug 20, 2024

I'm a bit confused why Github is telling me that there's still 1 change requested by @Anton-4, but it points me to this comment, and there's no "Resolve Conversation" button.

@BNAndras
Copy link
Member

The person requesting changes needs to approve and then you can merge.

@ageron
Copy link
Contributor Author

ageron commented Aug 21, 2024

Oh of course, thanks.

@ErikSchierboom ErikSchierboom requested a review from Anton-4 August 21, 2024 06:16

See https://exercism.org/docs/building/tracks/docs for more information. -->
To install Roc, please follow the instructions at [roc-lang.org/install](https://roc-lang.org/install).
You only need to install Roc, and optionally the editor Extensions/Plugins of your choice, you don't need to install Rust or Zig (which are used to build Roc itself).
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if any student would have expected to install Rust of Zig, that seems more like an implementation detail.
Maybe instead of this:

Suggested change
You only need to install Roc, and optionally the editor Extensions/Plugins of your choice, you don't need to install Rust or Zig (which are used to build Roc itself).
There are also some [Editor Extensions/Plugins](https://www.roc-lang.org/install#editor-extensions) that you might want to install.

Or something like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ErikSchierboom ,
I'm sorry I missed your last comments and I merged the PR. I'm happy to create another one to address your comments if you want.
@keiravillekode or @BNAndras suggested above that it might be good to clarify exactly what needs to be installed, since Roc's installation instructions give you several options (including installing Zig or Rust, in case you want to build Roc from source or create some Roc platforms).

Copy link
Member

Choose a reason for hiding this comment

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

Ah right! Nevermind, forget about my comment then.

docs/TESTS.md Show resolved Hide resolved
@ErikSchierboom
Copy link
Member

Sorry for the lengthy review process! That is partially caused by this being the first PR, and by this PR doing a lot of things :)

@ageron ageron merged commit 46ecc1a into exercism:main Aug 21, 2024
1 check passed
@ageron ageron deleted the start-docs-and-exercises branch August 21, 2024 08:52
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.

5 participants