READMEs: Import problem-specifications commits #354
Conversation
|
If you ask, the import was done by hand. I'll think about automating it later. It is intended that all commit messages link to the original problem-specifications PR, and that the author of each commit is the author of the problem-specifications commit. Let me know if I got any of that wrong. |
|
In case someone ask: No, it's not too bad if this gets squashed. My preference is to keep them separate commits so the rationale for each change is maintained, but I wouldn't complain if they get squashed into one commit with "regenerate READMEs from problem-specifications" since this is basically what this is doing (minus space-age). The strength of my preference is 4/10, so if someone has any stronger preference, you pick which way you like. |
|
I would prefer squashing these, the repo history would be cleaner in my opinion. |
|
OK, I defer. A reason that it is OK to defer is that if someone wants to see the individual commits, they at least have this PR to refer to, as long as GitHub continues to exist. |
|
The effect of Squash and Merge button on this PR seems to be (based on what I observed in exercism/haskell#603) that the resulting commit will have author of me (I submitted the PR) and committer of the one who merges. I'm fine with that. |
|
Since we will be using Squash and Merge on this PR: I will not be squashing the commits before merging because I prefer to be able to see the history of the individual changes, and since they are going to be squashed on merge, I need to have this PR as a fallback as where to see it. I plan to use something like the following for the commit message of the squashed commit. For this PR I see no need to violate my policy, so until it receives Approval (in the GitHub sense), I'm just going to keep lumping on problem-specifications changes into this same PR. |
The paragraphs rambled without newlines. They are now line wrapped to match the other exercises. exercism/problem-specifications#909
There was a missing article 'a' in the beginning. The phrasing "after the throw" is a little confusing. Does this mean either the 2nd or 3rd throws? No, it means they are knocked down by THIS throw. Update accordingly. exercism/problem-specifications#909
Overwrite replaces the oldest item in the buffer if the buffer is empty. Otherwise it is just like write. This commit documents what overwrite on a buffer with available space looks like and clarifies what the oldest element is after an overwrite operation succeeds. exercism/problem-specifications#889 exercism/problem-specifications#892
The final example in the crypto-square description is inconsistent with the [source of the exercise](http://users.csc.calpoly.edu/~jdalbey/103/Projects/ProgrammingPractice.html), the canonical data, and with itself. This change does a few things to correct that: - While the encoding method is called a "square code," we're really dealing with all sorts of rectangles, squares included. The description gets this right up until the last example, and this change clarifies that. - In both the source of the exercise and the canonical data, when a "chunk" is shorter than the length of a row, a blank space is added to the end of that chunk. This change fixes that in the final example. - The copy before the final example says that "spaces should be distributed evenly across the last `n` rows," but it doesn't explain how. This change specifies that the spaces should be added to the end of each chunk. Here's a related discussion: exercism/problem-specifications#356 exercism/problem-specifications#929
|
The kindergarten-garden commit was a mistake; that exercise is not in this track. |
| @@ -1,16 +1,18 @@ | |||
| # Proverb | |||
There was a problem hiding this comment.
As soon as exercism/problem-specifications#994 is merged, this will no longer be in sync.
|
@petertseng Given the amount of activity in the problem specifications, and the amount of work you do checking that various readmes match the generated version, is there a strong reason to put them in the repo at all? It's easy to imagine a system in which they're generated dynamically by merging relevant info from the problem-specifications repo and .meta; this seems superior to having to manually track and update them. |
There is a reason. I'll leave it up to you to determine whether exercism/meta#15 is considered strong.
https://github.com/exercism/trackler/blob/master/lib/trackler/implementation.rb#L51
I agree. |
|
Ok, it looks like it's exercism policy to have READMEs which are stored within the repo but dynamically generated. I still think it's a bad idea for humans to have to manage this stuff manually. I propose the following:
Sound plausible? |
|
The only thing I would advise before potentially starting this work is just making sure that Katrina did not already do the same, given the statement of intent in exercism/discussions#200 (comment) - I will assume that it's just a statement of intent and that nothing has been done yet, but still good to check. Here are some things you might take into account when making such a thing: I want the resulting commit to:
Feel free to convince me that I should not want any of those things, though. |
|
Good point. Digging around, I discovered exercism/meta#96, which seems to cover the scope of this work. However, there's still some discussion as to how much it should actually do; the thrust of the comments seems to be that it's simpler and generally more correct to trigger CI fails when something goes wrong than it is to attempt to generate things automatically. There are good points there, so let's reformulate again: what if we simply added a test which runs |
I will find this useful, especially for new exercises. We have seen today that we have not applied some README changes to new exercises. We have to be careful of situations such as: I make a change to exercise X, but in problem-specifications a change to exercise Y is made so my change to exercise X can't proceed until we match the change to exercise Y.
It could be as simple as adding a script; the location does not matter, only that it is listed in https://github.com/exercism/rust/blob/master/.travis.yml#L3 |
|
Is that actually a problem, though? If the default behavior of Pros: easy for contributors; keeps all exercise READMEs updated. |
I can't say for sure that the following will be a contributor's reaction to this, but I would think "why do I have to make these irrelevant changes in order to get the build to succeed? Why do I have to download this random tool that I don't even know I can trust, and who knows what will happen if I run it?" I would find it a bad experience for contributors. It may make life slightly harder for maintainers too, since if we intend to merge (say) a PR that adds an exercise and it has a commit for unrelated README change, we can no longer use Squash and Merge button since it doesn't make sense to make the two unrelated things into one commit, whereas we might otherwise be able to (and want to). |
|
Those are good points. In that case, what we want is for Travis to be able to detect which exercises are affected by the current PR, and only generate and compare READMEs for those. Something like for exercise in $(git diff --name-only master..$(git rev-parse --abbrev-ref HEAD) | grep exercises/ | cut -d'/' -f2 -s | sort -fu); do
# `$exercise` is a track name here, so Travis needs to run
# `configlet generate --only $exercise` and compare its output to the existing README
doneUsing a loop like that, we can fail if and only if a README already affected by the PR is now out of sync. |
|
The approach is acceptable because it doesn't start requiring PRs to have unrelated changes. Note that with this approach, our READMEs still may remain out of date for an unbounded amount of time, simply if there are no PRs affecting a README that goes out of date. Someone interested in that might try to devise a scheme that can help with that. |
petertseng
added
the
sync/readme
No description provided.