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

feat: match exercise order to book chapters #541

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

JuliaCao
Copy link
Contributor

@JuliaCao JuliaCao commented Sep 27, 2020

Exercises are not organized in order for rustlings watch and it was hard to follow the exercises because we always had to jump back and forth between chapters.

  1. Updated exercise order to match that of book chapters
  2. Added exercise to book chapter mapping table to exercise README.md

@JuliaCao JuliaCao changed the title Updated exercise order to match that of book chapters, and chore: match exercise order to book chapters Sep 27, 2020
@JuliaCao JuliaCao marked this pull request as ready for review September 27, 2020 06:09
@darnuria
Copy link
Contributor

darnuria commented Oct 3, 2020

Hi, this pr need some local testing? If so I can checkout and test myself I use rustling with students and feel some problems due to mismatch with book chapters/rustlings order.

@JuliaCao
Copy link
Contributor Author

JuliaCao commented Oct 3, 2020

Hi, this pr need some local testing? If so I can checkout and test myself I use rustling with students and feel some problems due to mismatch with book chapters/rustlings order.

Hello,

I have tested this locally. But feel free to double check :)

@AbdouSeck
Copy link
Contributor

Hi @JuliaCao, this is a very good idea. While I have not pulled the changes locally yet, I don't see how this would not be better than the current ordering of the exercises.

However, I have noticed that you've got a merge commit in your commits. This may have stemmed from a desire to keep your branch up-to-date with the main branch. In keeping with linear histories (among other things) in the project, would you mind using a rebase instead of a merge, please?
You may need to reset back to your own commit before the merge and then rebase from there. Please let me know if you have any questions.

Thanks,
Abdou

@JuliaCao
Copy link
Contributor Author

JuliaCao commented Dec 7, 2020

Hi Abdou,

Thanks for getting back on this. I've just rebased and updated my branch. Please take a look and let me know what you think.

Thanks,
Julia

Added exercise to book chapter mapping table to exercise README
Copy link
Contributor

@AbdouSeck AbdouSeck left a comment

Choose a reason for hiding this comment

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

@JuliaCao thanks for making the change. @fmoko this should be good to go.

@shadows-withal
Copy link
Member

I'm not sure whether this would necessitate a breaking change?

@AbdouSeck
Copy link
Contributor

@fmoko none that I could detect. The way the info.toml file de-serialized is hidden from the end users and there does not seem to be anything else in the source code relying on the order of the exercises within the code for the watch command. But I would not hurt to mention in the change logs that the ordering of the exercises was changed to match the Book's chapters.

Thanks,
Abdou

@shadows-withal
Copy link
Member

@AbdouSeck Yup, I think deciding whether it's a breaking change is mostly a semantic thing here, but I'll go with your judgement. Thanks!

@shadows-withal shadows-withal changed the title chore: match exercise order to book chapters feat: match exercise order to book chapters Dec 7, 2020
@shadows-withal shadows-withal merged commit 033bf11 into rust-lang:main Dec 7, 2020
@AbdouSeck
Copy link
Contributor

@JuliaCao @fmoko I really messed up here. The merged info.toml from this PR skipped 5 exercises:

 iterators1
collections1
collections2
collections3
collections4

This may have happened during rebase? Either way, I am happy to bring those back. But I wanted to give @JuliaCao the opportunity to take a stab at it since she's more comfortable with the ordering of the exercises?
Let me know what you think.

Thanks,
Abdou

@AbdouSeck AbdouSeck self-assigned this Dec 12, 2020
@JuliaCao
Copy link
Contributor Author

@AbdouSeck Sure. Let me take a look

@JuliaCao
Copy link
Contributor Author

Opened a new PR for a fix here #598

@AbdouSeck
Copy link
Contributor

@JuliaCao wonderful. I'll get on it ASAP. Thanks!

JuniousY pushed a commit to JuniousY/rustlings that referenced this pull request Jan 28, 2021
Added exercise to book chapter mapping table to exercise README
mccormickt pushed a commit to mccormickt/rustlings that referenced this pull request Mar 10, 2021
Added exercise to book chapter mapping table to exercise README
noiffion pushed a commit to noiffion/rustlings that referenced this pull request Aug 20, 2021
Added exercise to book chapter mapping table to exercise README
bugaolengdeyuxiaoer pushed a commit to bugaolengdeyuxiaoer/rustlings that referenced this pull request Dec 28, 2021
Added exercise to book chapter mapping table to exercise README
ppp3 pushed a commit to ppp3/rustlings that referenced this pull request May 23, 2022
Added exercise to book chapter mapping table to exercise README
dmoore04 pushed a commit to dmoore04/rustlings that referenced this pull request Sep 11, 2022
Added exercise to book chapter mapping table to exercise README
Spacebody pushed a commit to Spacebody/my-rustlings that referenced this pull request Nov 18, 2022
Added exercise to book chapter mapping table to exercise README
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.

4 participants