-
Notifications
You must be signed in to change notification settings - Fork 69
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
Komga implementation #943
Komga implementation #943
Conversation
Cleaned up where some variables lived and documented important code. Removed unused imports Templatized duplicated code. Switched to an unbounded channel to allow users to make any length of background task polling periods.
Looking good! Requested for a few small changes. Will take a better look when I get the time. Thanks! |
Let me know if I need to make any changes here. |
Let me know if there are any other concerns. |
Addressed a bug where some manga imported with a different number than is shown in the interface not sure what causes this but metadata.number is the authoritative number and is also the one that gets edited in the interface the original item.number appears to be the order in which they import into the series. |
Not sure about this, but is your rust formatter working? Maybe you have to install rust-analyzer. None of the rust file changes you made are formatted, it seems. |
It appears my IDE had the formatter disabled I believe this has been resolved. |
How does this functionality work? Does it just add the things which are tracked to the owned collection or is this doing an import of all books into the owned collect? |
As of now, it just works for ABS. It just adds all items in your library to the owned collection. Can I give you access to the Pro repo, and you can take a look at the code? |
Yeah that works I see no reason why we couldnt support that |
Sent you the invitation. This PR LGTM, i will merge it later today. |
Run CI.
Run CI.
Run CI.
Run CI.
should-run: ${{ steps.check.outputs.should-run }} | ||
should-release: ${{ steps.tag.outputs.should-release }} | ||
should-run: ${{ steps.set_outputs.outputs.should-run }} | ||
image-names: ${{ steps.set_outputs.outputs.image-names }} |
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.
@vnghia I have been trying to get this working for some time, but it seems that there is no way to publish a docker image from a PR (from a fork) since the GITHUB_TOKEN
granted to the workflow only has read permissions (despite specifying write explicitly in the action).
Do you know a solution to this?
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.
So the current flow never works for PRs. Only for PRs from a branch in the same repo.
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.
let me think about it. you can use env
instead of secrets
but it will print your env in the log
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.
But to actually inject the secrets into env, i would still need access to secrets. Which are not available in PRs from forks.
Introduced YankIntegrationWithCommit trait to address issue with passing the closure through a staticly allocated enum. Renamed the integration file to integration trait and moved all integrations traits to this file Changed visibility of all integrations to be pub(crate) since they dont need to be accessible anywhere else. Introduced proper error handling for improper integration types
@Jacob-Tate Is this ready to be merged? |
Yes this should be ready ive been running it for a few days without any issues. |
Will release this with v7 next week. Thanks for the contribution! |
This appears to be working well in my testing and properly handles mass marking as read as well as marking in progress reading correctly. Needed to add in a special case in misc for a progress update where the chapter is provided out of order as occasionally apps like mihon appear to update this information in random order. This occasionally resulted in the wrong chapter being marked as read.