-
Notifications
You must be signed in to change notification settings - Fork 2
Chapter and section revisions #3
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
Conversation
ezrichards
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall! A couple slight wording changes that I would make, but I think that this is a good start.
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
gregbell26
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know i wasnt initially invited, but it looks mostly good to me, just some things that I think should be adjusted.
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
…inkcspy into Chapter-and-Section-Revisions
gregbell26
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that one outstanding change from my last review, this looks good to me!
|
I think it might be worth tying the mutability stuff with lists in to what ethan talks about in the None section in #1 |
metesaka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finished up editing your recommendations
|
I found some more instances of pure functions. https://github.com/search?q=repo%3ACSCI128%2Fthinkcspy%20pure%20functions&type=code |
|
@TriHardStudios just FYI, .rst files are no longer used. I'll probably delete them sooner or later. But yes, it looks like there are still a few in pretext. |
ezrichards
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small changes, but looking really good overall!
robotuw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tentative approval for now. These changes look good but I'd like to see them compiled into the book, and I can't do that until Wednesday.
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
Co-authored-by: Ethan Richards <42894274+ezrichards@users.noreply.github.com>
To better emphasize the topics that have high confusion, several chapters and sections are revised. A review is appreciated. @robotuw @ezrichards