Uh oh!
There was an error while loading. Please reload this page.
robot-name approach: fix typos, minor rephasing/improvements - #4051
Conversation
- Fixed a few typos. - Rephrased here and there where I subjectively thought it useful. Please let me know if they're undesirable. - Expanded some contractions - this might help non-native speakers. - When another approach is mentioned, added link to it - hopefully this is useful rather than an distraction - I thought the explanation about the alternative to using the walrus operator was not clear, so rephrased and added a code snippet. I hope this is useful.
BethanyG
left a comment
There was a problem hiding this comment.
A few nits (some contractions you missed), and a few rewordings that came to mind as I reviewed. Overall, though -- this is great! This is certainly within what I had in mind, and I wouldn't mind if you had more to add or more edits (at least for the approaches I wrote 😄 ). Let me know if you prefer comments or "request changes". I typically do comments or approval and leave you to take my suggestions ... or not.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
kotp
left a comment
There was a problem hiding this comment.
Great start... a couple of nits for style.
| @@ -1,5 +1,5 @@ | |||
| # Mass Name Generation | |||
| We'd first have to generate all the possible names, shuffle them, and then use `next` (the simplest way) or maintain a `current_index` and get the name. | |||
There was a problem hiding this comment.
Style suggestion:
Blank line after headings (currently missing).
Tend to trailing white space, there are two occurrences where this happens... I will mention in review here.
There was a problem hiding this comment.
Would it be OK if I also added blank lines before/after the code snippets in the markdowns? It should not make a difference when rendered, but I find it easier to read and edit the markdown that way -- is that something that works for everybody or just my personal quirk?
There was a problem hiding this comment.
Would it be OK if I also added blank lines before/after the code snippets in the markdowns?
Please do. Not just your quirk: mine as well. Didn't do it on my first pass, because I didn't want to bog things down. But absolutely!
| def __init__(self): | ||
| self.reset() |
There was a problem hiding this comment.
| def __init__(self): | |
| self.reset() | |
| def __init__(self): | |
| self.reset() |
There is at least one other trailing white space lurking that is similar to the above...
There was a problem hiding this comment.
Thanks, I dealt with trailing whitespace and lines after headings in one commit.
…/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
…/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
…/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
…/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
…ent.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
…ent.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
…/content.md Co-authored-by: András B Nagy <20251272+BNAndras@users.noreply.github.com>
petrem
commented
Dec 21, 2025
Thanks everyone for comments and suggestions!
Thanks! There still are a few contractions. I find some of the more common ones, like "isn't" or "won't", to be useful. They "roll down well", and make the tone a bit of a less formal. But please let me know if the agreed style/wisdom is to not use contractions at all, or seldom, or ... While looking at the text again, I found this one that might not be grammatically correct, but as I didn't study grammar I'm not sure: ensure that it's not been previously used I think this should read, perhaps, "ensure that it hasn't been ...".
Thanks :-)
This may sound strange, but I don't think I actually know the difference. I'm not too familiar with github core reviews. I worked a lot with bitbucket and just very occasionally with github and gitlab. In general, please do as you think it is more appropriate or whatever is more convenient. |
BNAndras
commented
Dec 21, 2025
|
petrem
commented
Dec 21, 2025
oh, 'tis right, only I'd stopped for a second and thunk.. |
BethanyG
commented
Dec 21, 2025
@petrem - than you so much for taking this on! Give a head's up when you are "done done", and I can give things a quick once-over and then merge. 😄 |
petrem
commented
Dec 21, 2025
@BethanyG thank you for reviewing, I'm done (unless new evidence to the contrary is surfaced). |
kotp
commented
Dec 21, 2025
via email
On 25/12/21 03:26AM, PetreM wrote:
@petrem commented on this pull request.
> @@ -1,5 +1,5 @@
Would it be OK if I also added blank lines before/after the code snippets in
the markdowns? It should not make a difference when rendered, but I find it
easier to read and edit the markdown that way -- is that something that works
for everybody or just my personal quirk? Not only OK, it would be preferred. |
kotp
commented
Dec 21, 2025
via email
On 25/12/21 03:50AM, PetreM wrote:
@petrem commented on this pull request.
> def __init__(self):
self.reset()
Thanks, I dealt with trailing whitespace and lines after headings in one commit.
Thanks! |
BethanyG
commented
Dec 21, 2025
Pushing the big, green button! Thanks again, @petrem! 🚀 |
Uh oh!
There was an error while loading. Please reload this page.
…sm#4051) * robot-name approach: fix typos, minor rephasing/improvements - Fixed a few typos. - Rephrased here and there where I subjectively thought it useful. Please let me know if they're undesirable. - Expanded some contractions - this might help non-native speakers. - When another approach is mentioned, added link to it - hopefully this is useful rather than an distraction - I thought the explanation about the alternative to using the walrus operator was not clear, so rephrased and added a code snippet. I hope this is useful. * Update exercises/practice/robot-name/.approaches/mass-name-generation/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com> * Update exercises/practice/robot-name/.approaches/mass-name-generation/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com> * Update exercises/practice/robot-name/.approaches/mass-name-generation/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com> * Update exercises/practice/robot-name/.approaches/mass-name-generation/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com> * Update exercises/practice/robot-name/.approaches/name-on-the-fly/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com> * Update exercises/practice/robot-name/.approaches/name-on-the-fly/content.md Co-authored-by: BethanyG <BethanyG@users.noreply.github.com> * Update exercises/practice/robot-name/.approaches/mass-name-generation/content.md Co-authored-by: András B Nagy <20251272+BNAndras@users.noreply.github.com> * blank lines after headers and remove trailing whitespace * add blank lines around code snippets and expand one more contraction --------- Co-authored-by: BethanyG <BethanyG@users.noreply.github.com> Co-authored-by: András B Nagy <20251272+BNAndras@users.noreply.github.com>
Additionally, the phrase
seems misleading to me: calling
__init__from other methods seems to me significantly worse than calling other dunder methods, as it may lead to future bugs.Arguably, for tooling support, we should assign to
self.namein__init__()explicitly:but this looks ugly and confusing.