Sync exercise markdown files and metadata #662
Conversation
BNAndras
changed the title
BNAndras
force-pushed
the
sync-exercise-files
branch
from
2eed747 to
309383a
Compare
|
@exercism/julia, can I get a review? I pulled down the latest docs and metadata. |
|
Can you provide the exact code that you ran to do the sync so that I can confirm I get the same output without having to eyeball that huge diff? |
| Bob's conversational partner is a purist when it comes to written communication and always follows normal rules regarding sentence punctuation in English. | ||
| - **"Sure."** | ||
| This is his response if you ask him a question, such as "How are you?" | ||
| The convention used for questions is that it ends with a question mark. |
There was a problem hiding this comment.
This has poor grammar (questions is plural => "they" rather than "it"). Please consider opening a PR on the problem-spec repo.
| The convention used for questions is that it ends with a question mark. | |
| Questions end with a question mark. |
| - conversion and promotion of integers to rationals. Read the manual's chaper on [conversion and promotion](https://docs.julialang.org/en/v1/manual/conversion-and-promotion/) for details about how and why to do this. | ||
| - addition, subtraction, multiplication and division of two rational numbers | ||
| - absolute value, exponentiation of a given rational number to an integer power, exponentiation of a real number to a rational number | ||
| - comparison (less than, equals, etc.) of two rational numbers, comparison of a rational number and an integer | ||
|
|
||
| In Julia, there are fallback methods already defined for many arithmetic methods, comparisons, etc, especially for subtypes of `Real`. | ||
| You do not need to write your own method for `>=` if you have already defined `<`; nor do you need to define `^(::RationalNumber, ::Integer)` if you have provided a method for multiplication; etc. | ||
| You can examine the fallback methods for a given function by entering `methods(abs)` or `edit(abs, (RationalNumber, RationalNumber))` at the REPL. | ||
|
|
||
| Your implementation of rational numbers should always be reduced to lowest terms. For example, `4/4` should reduce to `1/1`, `30/60` should reduce to `1/2`, `12/8` should reduce to `3/2`, etc. To reduce a rational number `r = a/b`, divide `a` and `b` by the greatest common divisor (gcd) of `a` and `b`. So, for example, `gcd(12, 8) = 4`, so `r = 12/8` can be reduced to `(12/4)/(8/4) = 3/2`. | ||
| The reduced form of a rational number should be in "standard form" (the denominator should always be a positive integer). If a denominator with a negative integer is present, multiply both numerator and denominator by `-1` to ensure standard form is reached. For example, `3/-4` should be reduced to `-3/4` |
There was a problem hiding this comment.
There's Julia-specfic stuff in here that should be retained. Without these hints the exercise is harder and students will probably learn less.
There was a problem hiding this comment.
The first set of bullet points also contains content that should be retained:
- conversion and promotion of integers to rationals. Read the manual's chaper on [conversion and promotion](https://docs.julialang.org/en/v1/manual/conversion-and-promotion/) for details about how and why to do this.
- comparison (less than, equals, etc.) of two rational numbers, comparison of a rational number and an integer
IMO, it is important to keep hinting to the student that conversion and promotion is the first thing they should do. If you want to move this into the .append file then please find some way of making it more prominent.
Otherwise, maybe just revert the changes to the files in this directory.
There was a problem hiding this comment.
I'd rather move the Julia-specific information to an append as long as we're not duplicating information in the instructions. That's the suggested workflow in the Exercism docs and I don't see a good reason to deviate. Editing the instructions in place just delays the issue.
There was a problem hiding this comment.
I appreciate your work in revising this PR. Are you saying that you intend to move those two lines into a .append file?
As I say below, I think that's fine so long as the advice to start with conversion and promotion is made prominent (ideally even more prominent than it is now).
Re: reverting, you don't see a good reason to deviate from the suggested workflow.
The rational-numbers exercise, like several others on the Julia track, has a bunch of intentional differences from the upstream definitions in problem-specs. I think the julia-specific changes are good and that if we remove them it should be because they are bad for students or mentors, not because they are inconsistent with problem-specifications.
What is the cost of letting rational-numbers continue to differ from the problem-spec version? I don't work on this repo very much any more, is there a developer experience issue that I'm not seeing?
There was a problem hiding this comment.
Yeah, I've been making appends as we went through the changes.
Regarding deviation, I meant the website's docs say instructions.md and introduction.md should match what's in the problem specifications. configlet sync syncs the copy from problem specifications so if you add track-specific text to those two files, it'll be overwritten. configlet sync doesn't touch the .append files so any track-specific content there will be unaffected.
A problem with the appends is that they go beneath the upstream content on the site. Therefore, you can only clarify the upstream content, not modify text in it. Moving the Julia-specific edits to the bottom means the student loses some of their original context.
That said, using .appends simplifies track maintenance because it allows for easy syncing. This year, Erik and Jeremy are updating the upstream content almost every week.
|
|
I'll work on finishing this over the weekend. |
|
For rational-numbers, the append currently contains the Julia-specific content discussing fallback methods. I'm not seeing anything else that needs to be captured in the append. |
|
The append does not contain the extra bullet points that go under "implement the following...": https://github.com/exercism/julia/pull/662/files#r1470022604 Please either revert the changes to the rational numbers folder (contributors can just deal with configlet sync warning them about rational numbers) or find a way to include that information in a natural and high-profile way (I can't find a nice way of doing it, so I favour reverting the changes to that folder). |
|
There isn’t a way to include that information inline with an append. Rational Numbers isn’t a featured #48in24 exercise so I’m not expecting new upstream content in the near future. So we can keep our version but just be aware of any configlet syncs in the future. I’d rather use the latest upstream content and add the Julia-specific bits into the files since there’s generally a good reason for the docs to have been updated upstream. |
|
I'm fine with the julia specific instructions from those bullet points going into the .jl file instead. That could be a good place for them, otherwise, yes, please revert them and I'll merge this PR. |
BNAndras
force-pushed
the
sync-exercise-files
branch
from
b40219a to
5405bb1
Compare
|
I reverted the sync for the rational-numbers documentation. |
|
Thanks again for this contribution. Because this is a force push, I can't easily see what changed in the commit, but I'm just going to trust you on it and merge it. In future, please help me out by avoiding force pushes or rebases and making separate commits when responding to review. |
No description provided.