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

logs-logs-logs: update concept and exercise content #1707

Merged
merged 9 commits into from
Oct 2, 2021
Merged

logs-logs-logs: update concept and exercise content #1707

merged 9 commits into from
Oct 2, 2021

Conversation

sudomateo
Copy link
Contributor

@sudomateo sudomateo commented Sep 27, 2021

This patch adds the Runes concept and a logs-logs-logs exercise to Exercism's Go track. The Runes concepts teaches users UTF-8 encoding and shows that strings are a slice of bytes that can be read as bytes or characters (i.e., rune). The logs-logs-logs exercise tests that users know how to work with strings in Go.

This is part of the September 2021 sprint.

Closes #1594
Closes #1595
Closes #1596
Closes #1614

@sudomateo sudomateo changed the title logs-logs-logs: update exercise content logs-logs-logs: update concept and exercise content Sep 29, 2021
@sudomateo sudomateo marked this pull request as ready for review September 30, 2021 05:55
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.

Thanks a ton! I've left some comments on your PR. @junedev could you also take a look?

concepts/runes/about.md Outdated Show resolved Hide resolved
concepts/runes/.meta/config.json Outdated Show resolved Hide resolved
concepts/runes/about.md Outdated Show resolved Hide resolved
concepts/runes/links.json Show resolved Hide resolved
exercises/concept/logs-logs-logs/.docs/hints.md Outdated Show resolved Hide resolved
exercises/concept/logs-logs-logs/.meta/exemplar.go Outdated Show resolved Hide resolved
exercises/concept/logs-logs-logs/.meta/exemplar.go Outdated Show resolved Hide resolved
@@ -1,16 +1,100 @@
# Introduction
Copy link
Member

Choose a reason for hiding this comment

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

The introduction is currently quite long (same goes for the concept introduction.md document). Are there maybe places where you cut down a little on the information? It might be worth keeping the following two parts from the docs in mind:

  • The information provided should give the student just enough context to figure out the solution themselves.
  • Only information that is needed to understand the fundamentals of the concept and solve the exercise should be provided. Extra information should be left for the concept's about.md document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made some changes. Feel free to review.

Copy link
Member

@junedev junedev left a comment

Choose a reason for hiding this comment

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

Left some comments. In general I really like the story and exercise, great work!

concepts/runes/about.md Outdated Show resolved Hide resolved
concepts/runes/about.md Outdated Show resolved Hide resolved

## Runes and Strings

Since Go source code files are encoded using UTF-8, strings in Go are also encoded using UTF-8. That means strings in Go contain Unicode characters. Since a `rune` type is meant to represent a Unicode character, a string in Go is often referred to as a sequence of runes. Taking this one step further, since runes are stored as 1, 2, 3, or 4 bytes, a string can also be referred to as a sequence of bytes. Go uses slices to represent such sequences, and these slices can be iterated over using `range`.
Copy link
Member

Choose a reason for hiding this comment

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

The main takeaway of the section below should be that range for strings iterates over characters not bytes. Currently that is somewhat hidden. Besides making the text a bit clearer on this, I would recommend to change the first example to also include a special character so it becomes visible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated! Feel free to review.

concepts/runes/about.md Outdated Show resolved Hide resolved
concepts/runes/links.json Show resolved Hide resolved
exercises/concept/logs-logs-logs/.meta/exemplar.go Outdated Show resolved Hide resolved
exercises/concept/logs-logs-logs/.meta/exemplar.go Outdated Show resolved Hide resolved
exercises/concept/logs-logs-logs/.meta/exemplar.go Outdated Show resolved Hide resolved
exercises/concept/logs-logs-logs/.docs/instructions.md Outdated Show resolved Hide resolved

## Runes and Strings

Since Go source code files are encoded using UTF-8, strings in Go are also encoded using UTF-8. That means strings in Go contain Unicode characters. Since a `rune` type is meant to represent a Unicode character, a string in Go is often referred to as a sequence of runes. Taking this one step further, since runes are stored as 1, 2, 3, or 4 bytes, a string can also be referred to as a sequence of bytes. Go uses slices to represent such sequences, and these slices can be iterated over using `range`.
Copy link
Member

Choose a reason for hiding this comment

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

From the text I find it hard to understand that a string is a slice of bytes but is NOT a slice of runes because of the UTF8 encoding (but it can be converted to one as you explained elsewhere).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated! Feel free to review.

@sudomateo
Copy link
Contributor Author

Implemented most of the feedback. I wanted to finish all of it before calling it a night but my brain is just not cooperating. I know there's a push to have this implemented before October so I apologize for the delay here! I'll work on it again tomorrow after some rest.

@ErikSchierboom
Copy link
Member

@sudomateo Sure. Give us a shout when we can review the PR again.

@junedev junedev added status/awaiting-contributor This pull request is waiting on the contributor. x:size/large Large amount of work labels Oct 1, 2021
@sudomateo
Copy link
Contributor Author

@ErikSchierboom and @junedev this is ready for review! Thank you for your patience.

@junedev
Copy link
Member

junedev commented Oct 2, 2021

@sudomateo Thanks for the update. Could you have another look at 2 things before I merge this.

  1. It seems to me RuneCountInString is now used in the exemplar but not introduced in the introduction/concept docs. Since this function is a bit hard to find I think it's better to show it.
  2. It feels a bit like the order of the blocks in the about.md file got mixed up. The variable number of bytes information is now mentioned before it was introduced in the UTF8 section.

- Introduce `utf8.RuneCountInString` in the content.
- Reorder sections.
@sudomateo
Copy link
Contributor Author

Updated!

@junedev
Copy link
Member

junedev commented Oct 2, 2021

All good now @sudomateo, thanks! I am merging this now. If we want to make more improvements in the future (e.g. shortening the introduction even more) we can do another PR.

@junedev junedev merged commit ff8cb6d into exercism:main Oct 2, 2021
@sudomateo
Copy link
Contributor Author

Works for me! Thanks for the reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/awaiting-contributor This pull request is waiting on the contributor. x:size/large Large amount of work
Projects
None yet
3 participants