-
Notifications
You must be signed in to change notification settings - Fork 23
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
update v3 scheduler section #33
Conversation
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Co-authored-by: Danika-Dakika <[email protected]>
Just food for thought -- I know this page is big and requires a lot of edits, but it would be good to avoid expanding the scope of the changes with every new set of commits. (Perhaps it's even more important on a big edit like this?) Of course, there will always be times when an essential change doesn't make it into the initial PR -- like a big error that didn't catch your eye until the fifth time through the file, or lines that need to change to keep up with other edits. But we already play fast-and-loose with the concept of Do One Thing in these PRs, and adding new changes during the review stage makes it tough to track what has and hasn't been reviewed. |
This should be good to go without further bikeshedding. Note for me: If this is not merged until the next release, document weighted fuzz (load balancer) under Fuzz. |
I know what you are talking about but just in case you don't know, you can look at changes from only one commit instead of all. I also didn't know you were in the middle of review, I thought you had finished reviewing everything. |
I do know that -- and in groups of commits too. But that won't tell me if this is a new change to a section I haven't looked at, or an additional change to a section I have. |
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.
Sorry for the delayed review. There's one thing that needs changing:
@@ -7,7 +7,7 @@ scheduler](./the-anki-2.1-scheduler.md) ("v2"). | |||
|
|||
As of Anki/AnkiMobile 23.10, and AnkiDroid 2.17, the v3 scheduler is the default and only option. | |||
|
|||
On older versions, the scheduler can be changed in the preferences screen. | |||
On earlier versions, the scheduler can be changed from the preferences screen. |
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.
Note for future PRs: are changes like this line really improving legibility? They feel more like change for the sake of change, though I'm aware the fact that I'm a native English speaker may be biasing my perceptions.
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.
The preposition from felt more natural here. Although, I agree I could've kept the in in there.
As for the word old, I think it comes with a negative connotation oftentimes. Using earlier is neutral and is more clear. One might think, because they downloaded Anki a few months ago, it's probably not "old".
src/the-2021-scheduler.md
Outdated
|
||
Because exclusion is done when you click on a deck, the counts you see on the deck | ||
Because burying is done when you click on a deck, the counts you see on the deck |
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.
This is wrong. Cards are skipped during gathering if a sibling has been seen. No cards are buried until you actually answer the sibling.
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.
It wasn't very clear to me what is "exclusion" so I thought it's burying. Can you rewrite this?
Thank you! Please follow up with a new commit if you feel the wording still needs work. |
Dae, I'm confused about this one, "The actual burying still happens as you review cards".
This one doesn't apply to new cards right? As the burying happens right after gathering?