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

Remove deprecated Manga constructors #1499

Conversation

palaks-1
Copy link
Contributor

Remove deprecated Manga constructors to make sure authors field is propagated and author is derived from it

@dragonx943 dragonx943 requested a review from Koitharu February 25, 2025 08:22
@Koitharu
Copy link
Member

I believe it would be better to store authors as Set<String>:

  • If source require some id of author we should find it by name (like it implemented in MangaDex now)
  • User can provide author name directly
  • Author does not depends on source

@palaks-1
Copy link
Contributor Author

palaks-1 commented Feb 25, 2025

I believe it would be better to store authors as Set<String>:

  • If source require some id of author we should find it by name (like it implemented in MangaDex now)
  • User can provide author name directly
  • Author does not depends on source

@Koitharu not all sources have same capabilities like MangaDex to get id by name for example MangaNato where we can get authors manga with this path /author/story/fHxjaGluYV9yZWFkaW5n and there is no way to get id by name there but we can get it during details retrieval for manga and i believe there is quite a lot of such sources.
Yes they can pass author as a name in search filed but it will treated as a search query in Kotatsu app from my understanding and if we would have separate field to search by author then it can put empty key to it.
With MangaTag for author we will be able to get more sources to work with author search or we can have AuthorTag without source field in it (not sure how this source is used)
What do you think?

@Koitharu
Copy link
Member

I would like to merge this feature into master before Kotatsu v8 release so let's keep it string for a while

@palaks-1
Copy link
Contributor Author

I would like to merge this feature into master before Kotatsu v8 release so let's keep it string for a while

Ok then will rebase it to master and use Set<String>

@Koitharu
Copy link
Member

Don't rebase and don't worry about conflicts, I'll fix it when merge

I believe most manga sources provide some way to obtain author id by name. In all we need an ability to search by author string for global search across multiple sources

@palaks-1 palaks-1 force-pushed the feature/remove_deprecated_manga_constructors branch from cd7774e to 8de1dab Compare February 26, 2025 17:37
@palaks-1 palaks-1 changed the base branch from feature/search_query to master February 26, 2025 17:37
@palaks-1 palaks-1 force-pushed the feature/remove_deprecated_manga_constructors branch from 8de1dab to e9224db Compare February 26, 2025 17:49
@palaks-1 palaks-1 force-pushed the feature/remove_deprecated_manga_constructors branch from e9224db to 2a5f5bb Compare February 26, 2025 17:51
@palaks-1
Copy link
Contributor Author

palaks-1 commented Feb 26, 2025

Don't rebase and don't worry about conflicts, I'll fix it when merge

I believe most manga sources provide some way to obtain author id by name. In all we need an ability to search by author string for global search across multiple sources

I already rebased))
It was needed because originally this MR was raised into feature/search_query and based on feature/search_query so if you want to merge it in master it should have to be rebased to master
Otherwise it will bring feature/search_query commits to master and it didn't have your today Manga constractor refactoring a0168a7

@Koitharu Koitharu merged commit f6145bc into KotatsuApp:master Feb 27, 2025
1 check passed
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.

2 participants