search

open-spaced-repetition / fsrs-rs

FSRS for Rust, including Optimizer and Scheduler
https://crates.io/crates/fsrs
BSD 3-Clause "New" or "Revised" License
185 stars 19 forks source link

add `progress` #174

Closed AlexErrant closed 8 months ago

AlexErrant commented 8 months ago

Let's keep the discussion about this PR here.

asukaminato0721 commented 8 months ago 
 Caused by:
  failed to load source for dependency `burn`

Caused by:
  Unable to update /home/runner/work/fsrs-rs/burn/burn

Caused by:
  failed to read `/home/runner/work/fsrs-rs/burn/burn/Cargo.toml`

Caused by:
  No such file or directory (os error 2)

seems caused by sub-module or something else.

fsrs-rs/burn/burn/Cargo.toml

L-M-Sherlock commented 8 months ago

It's relied on https://github.com/open-spaced-repetition/burn via submodule:

image
AlexErrant commented 8 months ago

Hmm, a small noise of protest regarding the additional commits. Yes, they make the build in this repo pass. However that's not important since all that matters is the CI in fsrs-browser passing.

Worse, they greatly increase the chance of merge conflicts.

I've been trying to keep my changes to fsrs-rs small and minimal to make it super easy to merge main into fsrs-browser. With the most recent commits, any changes to any tests will cause a merge conflict that will constantly require manual work to merge. I was thinking about using a Github action like the one described here to automatically keep the fsrs-browser branch up to date with main. The conflicts in the tests will prevent this from happening. If we have a fsrs-browser branch that is automatically kept up to date with main, we could then have a cron-job that will automatically update the fsrs-browser repo with the latest fsrs-rs. That would be cool since then you wouldn't need to constantly make PRs to fsrs-browser. Any changes to fsrs-rs's main would automatically be propagated to fsrs-browser without any manual PRs. (Once I write some tests to prevent broken releases of course.)

If you really want to "delete" the tests, I recommend commenting them out with \* code *\ since it will make it unlikely to cause merge conflicts.

Actually, I think the preferred solution would be to add an exception for running CI for the fsrs-browser branch in this repo. Again, all that matters is the CI in the fsrs-browser repo.

Side note, prioritizing reduction of merge conflicts is also the reason why I haven't been fixing any warnings, though my editor is constantly screaming at me about all the unused code 😅

L-M-Sherlock commented 8 months ago

I haven't thought about that, sorry. Could we revert this PR?

By the way, FSRS-rs will be stable recently, because I will focus on the integration of the fsrs4anki helper add-on into Anki. So the back merge will happen less frequently.

Thanks to your great contributions again.

AlexErrant commented 8 months ago

It's all good! I didn't spell out any of my thoughts about this anywhere, so at least now we're on the same page!

dae commented 8 months ago

@L-M-Sherlock as a first step towards that, do you think you could write a quick outline of how you plan to implement them at one point? My main concern with integration into Anki is that we don't add a lot of processing time between card answers, so for example it would be good to build a cache of how many cards are due on each day, instead of scanning the database for due cards on each answer.

L-M-Sherlock commented 8 months ago

@dae, let's talk about it in https://github.com/ankitects/anki/issues/3116