Skip to content
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

Added loops section #1789

Merged
merged 2 commits into from
Feb 7, 2024
Merged

Added loops section #1789

merged 2 commits into from
Feb 7, 2024

Conversation

mani-chand
Copy link
Contributor

This is a contribution of a loops section for Comprehensive Rust.

Specifying ```Vec::<String>::new()``` explicitly indicates that variable is intended to be a vector containing String elements. This can be helpful for documenting the code and ensuring that variable is used correctly throughout the codebase.
Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@@ -8,51 +8,15 @@ There are three looping keywords in Rust: `while`, `loop`, and `for`:

## `while`
Copy link
Collaborator

Choose a reason for hiding this comment

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

To avoid students seeing an empty "table of contents" slide, let's include the first section (while) here in src/control-flow-basics/loops.md.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@@ -0,0 +1,16 @@
---
minutes: 5
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

The sub-slides can omit this timing information.

Copy link
Collaborator

Choose a reason for hiding this comment

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

^^ still needs to be done

src/control-flow-basics/loops/for.md Outdated Show resolved Hide resolved
@@ -0,0 +1,21 @@
---
minutes: 5
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this too.

@@ -0,0 +1,20 @@
---
minutes: 5
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this too.

Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

You'll also need a change to SUMMARY.md so that these new sub-slides actually appear in the output. You can test this using mdbook serve as seen in the README.md file.

Note, too, that if you install dprint as described in CONTRIBUTING.md, then dprint fmt will fix the formatting for you.

@@ -0,0 +1,16 @@
---
minutes: 5
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

^^ still needs to be done

minutes: 5
---

## `for`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a top-level heading for this sub-slide, so use #

@@ -0,0 +1,17 @@
## `loop`
Copy link
Collaborator

Choose a reason for hiding this comment

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

#

@@ -25,39 +25,8 @@ fn main() {

## `for`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these headings and links can be omitted -- students and instructors typically page through the course with the next and previous links, so the next link will take them to for, and on to loop.

@mani-chand
Copy link
Contributor Author

Ok. I will complete the changes.

@mani-chand
Copy link
Contributor Author

mani-chand commented Feb 7, 2024

I edited SUMMARY.md but slides in web page( in local) didn't change. Can you please check.

@mani-chand mani-chand requested a review from djmitche February 7, 2024 18:09
src/SUMMARY.md Outdated Show resolved Hide resolved
@mani-chand mani-chand requested a review from djmitche February 7, 2024 19:11
@mani-chand
Copy link
Contributor Author

I don't know what's wrong with cla/google .

image

@djmitche
Copy link
Collaborator

djmitche commented Feb 7, 2024

Oh, that's frustrating -- GitHub picked the wrong email address for me. I can fix it up before merging.

@mani-chand
Copy link
Contributor Author

Tommarow ,I will complete break and continue slide.

@djmitche djmitche enabled auto-merge (squash) February 7, 2024 19:31
@djmitche djmitche merged commit 345cf64 into google:main Feb 7, 2024
32 checks passed
@mani-chand mani-chand deleted the Head branch February 7, 2024 19:35
djmitche pushed a commit to mani-chand/comprehensive-rust that referenced this pull request Feb 15, 2024
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.

2 participants