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

Comprehensive Rust v2 #1073

Merged
merged 54 commits into from
Nov 29, 2023
Merged

Comprehensive Rust v2 #1073

merged 54 commits into from
Nov 29, 2023

Conversation

djmitche
Copy link
Collaborator

@djmitche djmitche commented Aug 11, 2023

I've taken some work by @fw-immunant and others on the new organization of the course and condensed it into a form amenable to a text editor and some computational analysis. You can see the inputs in course.py but the interesting bits are the output: outline.md and slides.md.

The idea is to break the course into more, smaller segments with exercises at the ends and breaks in between. So outline.md lists the segments, their duration, and sums those durations up per-day. It shows we're about an hour too long right now! There are more details of the segments in slides.md, or you can see mostly the same stuff in course.py.

This now contains all of the content from the v1 course, ensuring both that we've covered everything and that we'll have somewhere to redirect every page.

I'd appreciate feedback on:

  • The order of topics, at the level of segments (don't get too deep yet, this is still very rough)
  • Whether the 1080-minute time limit is appropriate
  • If so, what we might think of cutting (about 5%)

Fixes #1082.
Fixes #1465.

@djmitche
Copy link
Collaborator Author

This PR now contains the "new" course in src/, as derived through some automation. That automation is repeatable, and we should be able to rebase with it fairly easily, even where git doesn't see a file as having moved.

There's still a lot of editing to do, and I'll start in on that from day 1 forward.

@djmitche djmitche force-pushed the cr2-organization branch 4 times, most recently from 0592424 to 3f0feba Compare September 8, 2023 14:03
@djmitche djmitche force-pushed the cr2-organization branch 2 times, most recently from 5a54d73 to f4c9b1a Compare September 15, 2023 00:34
@djmitche
Copy link
Collaborator Author

I've got Day 1 morning and a bit of the afternoon ("Tuples and Arrays") edited together now. So far, rebasing hasn't been bad, as the upstream changes have been relatively straighforward and the sequence of commits in this PR helps guide Git to carry those changes through to the final product.

However, I'm starting to lose the faith that this is going to work out:

  • This PR is essentially un-reviewable, except by loading up the result in mdbook serve and flipping through it.
  • I'm the only one looking at this, so it's certain to be of a rough level of quality at best, and need some work before being taught
  • This is almost certainly going to be a nightmare for translators

I can keep working on this, and get to the end, but I'm worried that either it will be merged with "@djmitche did a lot of work so let's just merge it" resulting in a poor quality course until we can collectively improve it; or that review will take a long time and we'll end up basically forking the course.

I can see a few fixes for this:

  • Maybe it's just OK and I should just keep working.
  • Maybe we can find a way for a few people to collaborate so that the quality is higher when it lands.
  • Maybe we should embrace the fork and eventually land this in a v2 branch that we can teach from and improve for a while before making the switch.

Thoughts?

@djmitche
Copy link
Collaborator Author

Now rebased onto main, and with all of day-1 complete. That was probably ~8 hours work (mostly on planes to/from RustConf), so it could be a while until I finish the other two days. That said, days 2 and 3 changed less, so perhaps a little faster.

@djmitche djmitche force-pushed the cr2-organization branch 2 times, most recently from e4583e2 to b5723cc Compare September 19, 2023 23:25
@mgeisler
Copy link
Collaborator

Hey @djmitche,

I haven't given this as much time as I should — I hope we can look at it in a sync meeting soon.

One thing to discuss is how this will impact the translations (so you should make sure @henrif75 is aware of this — he can help by preemptively having our Localization team do translations and have them reviewed by the many Rust experts).

My thinking right now is that we should implement google/mdbook-i18n-helpers#16 before we start publishing a new version of the course. We don't have anyone working on this issue yet, so that will require some thought.

@djmitche
Copy link
Collaborator Author

Regarding translations, I think that should be "not too bad" -- I've tried to not unnecessarily rewrite text that is appropriate to the new slide. So I would hope that many of the translated messages carry over without issue. Maybe as the work here finishes (with all three days complete) we can look at the number of mismatches, fuzzy matches, and exact matches and make a plan from there.

Copy link
Collaborator

@fw-immunant fw-immunant left a comment

Choose a reason for hiding this comment

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

Reviewed day 1; lots of comments inline.

src/SUMMARY.md Outdated Show resolved Hide resolved
src/SUMMARY.md Outdated Show resolved Hide resolved
src/hello-world/variables-and-values.md Outdated Show resolved Hide resolved
src/hello-world/variables-and-values.md Outdated Show resolved Hide resolved
src/hello-world/arithmetic.md Outdated Show resolved Hide resolved
src/hello-world/exercise.md Outdated Show resolved Hide resolved
src/language-features/casting.md Outdated Show resolved Hide resolved
src/SUMMARY.md Outdated Show resolved Hide resolved
src/user-defined-types/let-control-flow.md Outdated Show resolved Hide resolved
src/std-types/exercise.md Outdated Show resolved Hide resolved
@mgeisler
Copy link
Collaborator

Regarding translations, I think that should be "not too bad" -- I've tried to not unnecessarily rewrite text that is appropriate to the new slide. So I would hope that many of the translated messages carry over without issue.

Awesome, that is a relief to hear! I have a suggestion here: we should extract the messages from this branch ~weekly and msgmerge the result into the existing translations (or into the most active translations). That way @henrif75 can ask our Localization team (and community translators) to translate the messages while you work with @fw-immunant and others on the content.

I'm reluctant to add special code for this into mdbook-xgettext since the translation infrastructure is used by multiple projects now. But we could update all translations weekly and tell people not to do further msgmerge runs themselves.

@djmitche djmitche force-pushed the cr2-organization branch 2 times, most recently from da48bf1 to bbe451a Compare October 2, 2023 17:57
@djmitche djmitche linked an issue Nov 21, 2023 that may be closed by this pull request
@djmitche djmitche linked an issue Nov 21, 2023 that may be closed by this pull request
@djmitche
Copy link
Collaborator Author

UPDATED:

  • Reorganized to put borrowing and slices closer together
  • Doubled time for most exercises (based on feedback)
  • Increased time for std-{types,traits} (based on feedback)
  • Updated course guidelines to suggest 6 instructional hours per day (the course now has more breaks and more time for exercises, so this is very much not 6 hours of lecture)
  • Split into 4 days

The new outline has the following timings:

Fundamentals: 21 hours and 35 minutes: 2 hours and 25 minutes shorter than target 24 hours
Sessions:
  Day 1 Morning: 3 hours: right on time
    Welcome: 5 minutes
    Hello, World: 20 minutes
    Types and Values: 1 hour and 5 minutes
    Control Flow Basics: 1 hour
  Day 1 Afternoon: 2 hours and 55 minutes: 5 minutes shorter than target 3 hours
    Welcome: 0 minutes
    Tuples and Arrays: 1 hour
    References: 50 minutes
    User-Defined Types: 50 minutes
  Day 2 Morning: 3 hours and 15 minutes: 15 minutes OVER TARGET 3 hours
    Welcome: 3 minutes
    Pattern Matching: 50 minutes
    Methods and Traits: 1 hour and 5 minutes
    Generics: 45 minutes
  Day 2 Afternoon: 3 hours: 2 minutes shorter than target 3 hours
    Welcome: 0 minutes
    Standard Library Types: 1 hour and 10 minutes
    Standard Library Traits: 1 hour and 40 minutes
  Day 3 Morning: 2 hours and 15 minutes: 45 minutes shorter than target 3 hours
    Welcome: 3 minutes
    Memory Management: 1 hour and 10 minutes
    Smart Pointers: 45 minutes
  Day 3 Afternoon: 2 hours and 20 minutes: 40 minutes shorter than target 3 hours
    Welcome: 0 minutes
    Borrowing: 1 hour
    Slices and Lifetimes: 1 hour and 10 minutes
  Day 4 Morning: 3 hours: 4 minutes shorter than target 3 hours
    Welcome: 3 minutes
    Iterators: 45 minutes
    Modules: 45 minutes
    Testing: 55 minutes
  Day 4 Afternoon: 2 hours: 1 hour and 5 minutes shorter than target 3 hours
    Welcome: 0 minutes
    Error Handling: 45 minutes
    Unsafe Rust: 1 hour and 5 minutes

I'm comfortable with the distribution of content like this, until we've actually taught the course and gotten some harder data for how long each segment takes, at which point we can consider moving things around a bit more to even out the days. In particular, I've left the last day an hour short in case things slip!

djmitche and others added 3 commits November 28, 2023 17:30
- Use full stop at end of each item.
- Use ‘:’ instead of ‘-’.
- Use tight list spacing.
@mgeisler mgeisler enabled auto-merge (squash) November 29, 2023 15:17
Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

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

Congratulations Dustin! Amazing work here!

@mgeisler mgeisler merged commit 6d19292 into google:main Nov 29, 2023
31 checks passed
djmitche added a commit to djmitche/comprehensive-rust that referenced this pull request Nov 29, 2023
I forgot to remove this in google#1073 before merging.
@djmitche djmitche mentioned this pull request Nov 29, 2023
djmitche added a commit that referenced this pull request Nov 29, 2023
I forgot to remove this in #1073 before merging.
mgeisler added a commit that referenced this pull request Nov 30, 2023
This incorporates the new Chromium material from #1479 as well as the
reshuffle and rewrite from #1073.
mgeisler added a commit that referenced this pull request Nov 30, 2023
This incorporates the new Chromium material from #1479 as well as the
reshuffle and rewrite from #1073.

This is a clean update without any additional changes. I’ll un-fuzzy
the string later.
mgeisler added a commit that referenced this pull request Dec 2, 2023
This incorporates the new Chromium material from #1479 as well as the
reshuffle and rewrite from #1073.

This is a clean update without any additional changes. I’ll un-fuzzy the
string later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants