Skip to content

Clarify Rust indices from 0 in saddle-points and regenerate READMEs#900

Merged
petertseng merged 1 commit intoexercism:masterfrom
workingjubilee:jubilee-rust-indexes-at-0
Nov 2, 2019
Merged

Clarify Rust indices from 0 in saddle-points and regenerate READMEs#900
petertseng merged 1 commit intoexercism:masterfrom
workingjubilee:jubilee-rust-indexes-at-0

Conversation

@workingjubilee
Copy link
Copy Markdown
Contributor

@workingjubilee workingjubilee commented Oct 25, 2019

Closes #791

Of the 5 choices in that issue, results of the discussion were (effectively):
"Do not do 2."
"Do 2 or 3 to be consistent with Rust."
"Do 3 for now since it is easy, then 1 later."
"1 would be possible using a little more code."

As option 2 effectively has a negative vote, and 3 has two votes for, modifying hints.md comes out as the maintainer-preferred solution. This PR implements that and regenerates the README appropriately. Or rather, READMEs. While doing so, I discovered two other READMEs were dated and their latest version was more readable, so I included them in this PR.

@coriolinus
Copy link
Copy Markdown
Member

Hello @workingjubilee, thanks for contributing this!

Travis has failed because it believes that the saddle-points README is out of date. You can see the failure for yourself by clicking the "Details" button to the right of the Travis status indicator, then clicking through to the first failed suite.

Would you please re-pull your copy of the problem-specifications repo and then regenerate the READMEs again? That should resolve the Travis issue unless something particularly weird is going on.

@workingjubilee workingjubilee force-pushed the jubilee-rust-indexes-at-0 branch from 24f50d3 to 845948d Compare October 25, 2019 07:34
@workingjubilee
Copy link
Copy Markdown
Contributor Author

Whoops! I failed to regenerate again with configlet after I made a teensy tiny change in-repo, actually. Fixed.

Copy link
Copy Markdown
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

Sure! I am good with this solution to #791. Note that you cannot say "issue" in the line "Closes issue #791", so please remove that word. Then you will see the underline under the word "Closes" as expected.

I would like that the READMEs for other exercises either:

  • Do not go in this PR at all, or
  • Go in this PR, each as one commit, with links to the appropriate problem-specifications PRs that caused the corresponding change in problem-specifications.

It makes sense because, well, each commit should do a single thing, and changing other exercises's READMEs isn't to do with indexing in saddle-points. No preference as to which of those two plans of action is taken.

@workingjubilee
Copy link
Copy Markdown
Contributor Author

workingjubilee commented Oct 30, 2019

'kay! Looking around at the other exercises for the READMEs that got touched, it seems like they might need more updates anyways, so I'm just going to withdraw that change from this PR and then PR in some full updates. 💖

@workingjubilee workingjubilee force-pushed the jubilee-rust-indexes-at-0 branch from 845948d to 9d0cc3e Compare October 30, 2019 20:28
Copy link
Copy Markdown
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

Ready to go, thank you! Other maintainers have about 48 hours to object.

@petertseng petertseng merged commit ff8efe1 into exercism:master Nov 2, 2019
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.

saddle-points: Reconcile 1-based indices in README with 0-based indices in tests

3 participants