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

Fix #409 #411

Merged
merged 1 commit into from
Mar 24, 2023
Merged

Fix #409 #411

merged 1 commit into from
Mar 24, 2023

Conversation

ShootingKing-AM
Copy link
Member

@ShootingKing-AM ShootingKing-AM commented Feb 24, 2023

Suggested working fix to Issues #409 #407 #413

@Ali-C-Ila
Copy link

Thanks for this!
However, as the auther of this plugin @Pseudonium has gone dark for over two years, I find it unlikely that this will ever get merged.
I sincerely wish that if you (or anyone) have the passion for it, you can fork and maintain the code in the future.

@ShootingKing-AM
Copy link
Member Author

ShootingKing-AM commented Feb 25, 2023

Thanks for this! However, as the auther of this plugin @Pseudonium has gone dark for over two years, I find it unlikely that this will ever get merged. I sincerely wish that if you (or anyone) have the passion for it, you can fork and maintain the code in the future.

Ok, i made a release for this fix in my fork is you are interested. Added CI as well.
https://github.com/ShootingKing-AM/Obsidian_to_Anki/releases/tag/3.4.2

Maybe I will push all the changes on my fork onto this (CI and CodeQl changes) when pseudo(seems like he is doing his thesis these days) is back

@shoudeyunkaijianyueming

Want to add folders that exclude synchronization, similar to this branch
https://github.com/aviral-batra/Obsidian_to_Anki

@shoudeyunkaijianyueming

At the same time, is it possible to increase the function of reverse synchronization from anki to obsidian, thank you, there is currently no.

@ShootingKing-AM
Copy link
Member Author

At the same time, is it possible to increase the function of reverse synchronization from anki to obsidian, thank you, there is currently no.

Umm.. this PR is related to only to the fix mentioned. Yes its possible to increase reverse sync function, maybe you can open a new issue in my fork ? and maybe we can request @aviral-batra to make a PR to my fork ?

@shoudeyunkaijianyueming
Copy link

At the same time, is it possible to increase the function of reverse synchronization from anki to obsidian, thank you, there is currently no.

Umm.. this PR is related to only to the fix mentioned. Yes its possible to increase reverse sync function, maybe you can open a new issue in my fork ? and maybe we can request @aviral-batra to make a PR to my fork ?
Ok, I raised the question under your issues, it's my reason, I'm a newbie, sorry, the location of the question is wrong, and aviral-batra has not been updated for a long time (although I am using this now, the developer may give up development).

@shoudeyunkaijianyueming
Copy link

image

I remember, I didn't find the issue location of your copy library at that time.

@ShootingKing-AM
Copy link
Member Author

image

I remember, I didn't find the issue location of your copy library at that time.

Oops ! I forgot to enable them. You can find them now.
But please note a point, I wish to maintain the addon - meaning if there are any issues with existing functionality like this PR then I would like to fix it.
Sadly, I too, right now, don't have the time to develop it - meaning add newer functionality. But yes Pull requests with new functionality are welcome.

@Pseudonium
Copy link
Collaborator

Aha, so this is the potential fix... let me take a look through it now

@Pseudonium
Copy link
Collaborator

Phew, just a small change in the code then. Literally one line. Amazing! And yes, based on the console error, this should fix things...

Now, I just gotta figure out how releasing updates works again... once I do that, hopefully can get this merged today and push out an update.

@Pseudonium
Copy link
Collaborator

Ah, yeah, and sorry for going dark for so long... I guess I got very busy with uni and knew that this code probably had all sorts of bad practices I'd rather not touch with a 10-foot pole. So my attitude kinda became "once I find an issue that starts affecting my workflow, I'll fix it ASAP". I've been using the plugin basically daily anyway so thought that I'd catch one quickly once any cropped up. Though, judging by the fact that this is issue 409, maybe not.

@aviral-batra
Copy link

reason, I'm a newbie, sorry, the location of the question is wrong, and aviral-batra has not been updated for a long time (

Hi sorry, I haven't used obsidian to anki for a while but I re-opened my pull request with all of the updates that I made based on a bunch of pull requests. What was your question exactly I'm not sure I understand?

@ShootingKing-AM
Copy link
Member Author

ShootingKing-AM commented Mar 24, 2023

Heyyy @Pseudonium !! Atlast you are back now :3 yeah uni times are busy times :3
Good to have you back :3 Did you get my mail ?

As a proof of this working i have implemented e2e tests for all features. You can check the results here : ShootingKing-AM#52 (ShootingKing-AM#52 (comment))

@aviral-batra
Copy link

Heyyy @Pseudonium !! Atlast you are back now :3 yeah uni times are busy times :3 Good to have you back :3 Did you get my mail ?

As a proof of this working i have implemented e2e tests for all features. You can check the results here : ShootingKing-AM#52 (ShootingKing-AM#52 (comment))

Did you want me to open a pull req to your repo for my changes?

@Pseudonium
Copy link
Collaborator

Ah, so it was you who wrote the email! I haven't got a chance to read it as of yet..

So here's what I'm thinking - for now, I just wanna make as minimal changes to the codebase as possible, so I guess I can just pull this in, and then make a new release etc.

@Pseudonium Pseudonium merged commit d67cf3a into ObsidianToAnki:master Mar 24, 2023
@ShootingKing-AM
Copy link
Member Author

Heyyy @Pseudonium !! Atlast you are back now :3 yeah uni times are busy times :3 Good to have you back :3 Did you get my mail ?
As a proof of this working i have implemented e2e tests for all features. You can check the results here : ShootingKing-AM#52 (ShootingKing-AM#52 (comment))

Did you want me to open a pull req to your repo for my changes?

Yes. but not now, since @Pseudonium is back, you can tidy up your PR right here.

Ah, so it was you who wrote the email! I haven't got a chance to read it as of yet..

Yup 😁

So here's what I'm thinking - for now, I just wanna make as minimal changes to the codebase as possible, so I guess I can just pull this in, and then make a new release etc.

Yup but CI is going to fail i guess.

@Pseudonium
Copy link
Collaborator

Ah, ok, so what specifically would fail?

@ShootingKing-AM
Copy link
Member Author

Ah, ok, so what specifically would fail?

#412

@Pseudonium
Copy link
Collaborator

Ok, so that's good now, at least. Anything else I should do before the release?

@ShootingKing-AM
Copy link
Member Author

Good for now. You can release a minor version now :)

@ShootingKing-AM ShootingKing-AM deleted the patch-1 branch March 24, 2023 17:22
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.

5 participants