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

Regular expression concept with exercise #2159

Merged
merged 14 commits into from
Apr 8, 2022

Conversation

norbs57
Copy link
Contributor

@norbs57 norbs57 commented Apr 6, 2022

Fixes #2037

  • Concept regular expressions (in Go)
  • Concept exercise

The first four tasks in the concept exercise are based on tasks in the corresponding C# exercise.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2022

Dear norbs57

Thank you for contributing to the Go track on Exercism! 💙
You will see some automated feedback below 🤖. It would be great if you can make sure your PR covers those points. This will save your reviewer some time and your change can be merged quicker.

  • 📜 The following files usually contain very similar content.

    • concepts/<concept>/about.md
    • concepts/<concept>/introduction.md
    • exercises/concept/<exercise>/.docs/introduction.md

    Please check whether the changes you made to one of these also need to be applied to the others.

  • 🧦 If you changed the function signature or the function comment in the exemplar file or the stub file (<exercise>.go), make sure the change is applied to both files.

  • ✍️ If your PR is not related to an existing issue (and is not self-explaining like a typo fix), please make sure the description explains why the change you made is necessary.

  • 🔤 If your PR fixes an easy to identify typo, if would be great if you could check for that typo in the whole repo. For example, if you found Unicdoe, use "replace all" in your editor (or command line magic) to fix it consistently.

Dear Reviewer/Maintainer

  • 📏 Make sure you set the appropriate x:size label for the PR. (This also works after merging, in case you forgot about it.)

  • 🔍 Don't be too nit-picky. If the PR is a clear improvement compared to the status quo, it should be approved as clear signal this is good to be merged even if the minor comments you might have are not addressed by the contributor. Further improvement ideas can be captured in issues (if important enough) and implemented via additional PRs.

  • 🤔 After reviewing the diff in the "Files changed" section, take a moment to think about whether there are changes missing from the diff. Does something need to be adjusted in other places so the code or content stays consistent?

Automated comment created by PR Commenter 🤖.

@norbs57 norbs57 force-pushed the regular-expression-concept branch 6 times, most recently from f1f3282 to 645c2d5 Compare April 7, 2022 12:11
Fixes exercism#2037
* Concept: regular expressions (in Go)
* Concept exercise

The first four tasks in the concept exercise are based on tasks in the corresponding C# exercise.
@norbs57 norbs57 force-pushed the regular-expression-concept branch from 645c2d5 to 6b34ed2 Compare April 7, 2022 12:22
Copy link
Member

@andrerfcsantos andrerfcsantos left a comment

Choose a reason for hiding this comment

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

I really like this PR! The majority of the changes suggested are aimed at making some explanations a bit more clear.

I suggested the changes to about.md, but of course they should also be reflected in the other files where the same text is present.

concepts/regular-expressions/.meta/config.json Outdated Show resolved Hide resolved
concepts/regular-expressions/.meta/config.json Outdated Show resolved Hide resolved

The [syntax][regexp-syntax] of the regular expressions accepted is the same general syntax used by Perl, Python, and other languages.

All characters are UTF-8-encoded code points.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
All characters are UTF-8-encoded code points.
In Go, you can use UTF-8 to build your regular expressions.

I think this sentence is more useful.

Copy link
Contributor Author

@norbs57 norbs57 Apr 8, 2022

Choose a reason for hiding this comment

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

I have changed this to "Both the search patterns and the input texts are interpreted as UTF-8. ". Is that any better?

concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
exercises/concept/parsing-log-files/.meta/exemplar.go Outdated Show resolved Hide resolved
exercises/concept/parsing-log-files/.meta/exemplar.go Outdated Show resolved Hide resolved
exercises/concept/parsing-log-files/parsing_log_files.go Outdated Show resolved Hide resolved
exercises/concept/parsing-log-files/parsing_log_files.go Outdated Show resolved Hide resolved
@andrerfcsantos andrerfcsantos added x:size/medium Medium amount of work x:size/large Large amount of work status/awaiting-contributor This pull request is waiting on the contributor. and removed x:size/medium Medium amount of work labels Apr 7, 2022
@norbs57 norbs57 force-pushed the regular-expression-concept branch 2 times, most recently from 26d5c25 to 357fd49 Compare April 8, 2022 18:14
...

Co-Authored-By: André Santos <andrerfcsantos@gmail.com>
@norbs57 norbs57 force-pushed the regular-expression-concept branch from 357fd49 to 914d9bc Compare April 8, 2022 18:18
@norbs57 norbs57 requested a review from andrerfcsantos April 8, 2022 18:30
Copy link
Member

@andrerfcsantos andrerfcsantos left a comment

Choose a reason for hiding this comment

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

More changes to the about.md - apply to the other relevant files too.

After that, I think this is ready to be merged :)

concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
concepts/regular-expressions/about.md Show resolved Hide resolved
concepts/regular-expressions/about.md Show resolved Hide resolved
concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
concepts/regular-expressions/about.md Outdated Show resolved Hide resolved
exercises/concept/parsing-log-files/.docs/hints.md Outdated Show resolved Hide resolved
norbs57 and others added 10 commits April 8, 2022 23:36
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
Co-authored-by: André Santos <andrerfcsantos@gmail.com>
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
Development

Successfully merging this pull request may close these issues.

Implement new Concept Exercise: Regular Expressions
2 participants