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

Override book's title if Title is set in XML #532

Merged
merged 3 commits into from Jun 10, 2023

Conversation

vinhtq115
Copy link
Contributor

In ComicRack XML Schema, there is a Title field that is not being used by KCC. In this PR, I changed so that if this field is set, it will automatically be used.

Also, pyinstaller is added to requirements.txt for building on M1 to avoid pyinstaller: command not found.

@darodi
Copy link
Collaborator

darodi commented Jun 7, 2023

@vinhtq115
Thanks for your PR.

imho, we shouldn't add pyinstaller to requirements.txt
It is only needed for building an executable.
It is not needed when using the project from source
and in docker images, for example, it would add unneeded dependencies.

@darodi darodi self-requested a review June 7, 2023 19:18
@darodi darodi added the Enhancement New feature or request label Jun 7, 2023
@vinhtq115
Copy link
Contributor Author

I have removed pyinstaller from requirements.txt. Since the building instruction for M1 did not mention about pyinstaller and appdmg, I suggest adding a Wiki page for users who want to build the binary themselves.

@darodi
Copy link
Collaborator

darodi commented Jun 9, 2023

@vinhtq115

https://github.com/ciromattia/kcc/wiki/How-to-build-M1

@vinhtq115
Copy link
Contributor Author

@darodi I have updated the wiki page to be more specific.

@darodi darodi merged commit 935727c into ciromattia:master Jun 10, 2023
@darodi darodi added this to the 5.6.3 milestone Jun 10, 2023
@darodi
Copy link
Collaborator

darodi commented Jun 10, 2023

Thanks for the PR

axu2 pushed a commit to axu2/kcc that referenced this pull request Jun 28, 2023
* Allow overriding title if it is set in XML
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants