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

Rectify retrieval of extant Helsingin Sanomat strips … #105

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

ExTechOp
Copy link

@ExTechOp ExTechOp commented Nov 3, 2024

… (https://www.hs.fi/sarjakuvat/), also checked and corrected their metadata. Also rectify retrieval for "Anonyymit Eläimet" (anonyymitelaimet) and "Jäätävä Spede" (jaatavaspede) even though these strips have finished.

This PR addresses 2/3 of issue #103 and supersedes PR #104.

…ne go,

and also checked and corrected their metadata.
I also updated retrieval for "Anonyymit Eläimet" (anonyymitelaimet) and
"Jäätävä Spede" (jaatavaspede) even though these strips have finished.
@Olf0
Copy link

Olf0 commented Nov 3, 2024

Thank you for the nice, sounding branch name: While this technically makes no difference, when handling multiple PRs from different people at the same time (I wish that would be the case for Daily Comics) this is really helpful.

I know that "Anonyymit Eläimet" (anonyymitelaimet) and "Jäätävä Spede" (jaatavaspede) were concluded, hence I considered deleting them. Keeping them is fine, if old strips are repeated (as the English language "Calvin and Hobbes"), but makes no sense if only the same old comic strip is shown. I assume you have checked that, correct?

BTW (nitpick), commit messages should be written in present tense (I did not invent that, but after a while it started making sense for me): I will rectify that.

P.S.: The GitHub web-frontend makes it really easy to use Git without using the git command and without working on a local repository: You simply start working in the original repository, e.g. editing, deleting or adding files. GitHub's web-frontend will then guide you accordingly.
Hence there is no reason to delete your clone of the original repository, unless you no longer intend to work on that project.

P.P.S.: Also do not mind about posing PRs with multiple commits, or even commits which correct older commits in this PR: All commits in a PR can be squashed into a single commit when merging (and I usually do that).

@Olf0 Olf0 changed the title Corrected retrieval for all Helsingin Sanomat (www.hs.fi) strips Rectify retrieval for all extant Helsingin Sanomat (www.hs.fi) strips Nov 3, 2024
@Olf0 Olf0 changed the title Rectify retrieval for all extant Helsingin Sanomat (www.hs.fi) strips Rectify retrieval for all extant Helsingin Sanomat strips … Nov 3, 2024
@Olf0 Olf0 changed the title Rectify retrieval for all extant Helsingin Sanomat strips … Rectify retrieval for extant Helsingin Sanomat strips … Nov 3, 2024
@Olf0 Olf0 changed the title Rectify retrieval for extant Helsingin Sanomat strips … Rectify retrieval of extant Helsingin Sanomat strips … Nov 3, 2024
Copy link

@Olf0 Olf0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Olf0 Olf0 merged commit ca74137 into sailfishos-applications:devel Nov 3, 2024
1 check passed
@Olf0
Copy link

Olf0 commented Nov 3, 2024

I would appreciated very much, if you consider implementing the (three, AFAICS) HS-comics which are not yet supported by Daily Comics in the table of HS-comics. You definitely have the skills to do that: The extract.js is the same, most metadata is in aforementioned table, it only additionally needs a "very typical strip, displaying the main character(s) and / or setting" and an image representing the comic (as described in detail in the "new comic guide").

@ExTechOp ExTechOp deleted the helsingin_sanomat branch November 4, 2024 07:54
@Olf0 Olf0 self-assigned this Dec 7, 2024
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