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

add script to check if lead picture is not equal the favicon and if t… #1

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Strubbl
Copy link
Contributor

@Strubbl Strubbl commented Mar 19, 2023

…he lead picture number is less than two

This is just a safety net to not get the favicon as share image as it was in past. Maybe we can add this anyhow so that it runs before the great notifying starts?

@call-me-matt
Copy link
Owner

good idea! but maybe we can integrate this into our python script so we make sure its not forgotten to be executed. saves us also from administrating the variables (like languages) in two destinations.

@Strubbl
Copy link
Contributor Author

Strubbl commented Mar 20, 2023

Yes, that should be possible. I just integrated it into the bash wrapper script. Before i go on detecting the languages, please tell me if it makes sense like this? For detecting languages i could do it the bash way (starting with ls configs/weekly_??.yaml and so on).

Or do you really want this to be implemented in pure python without any bash?

feed="https://weeklyosm.eu/feed"
article_id=$(curl -Ls "$feed" | grep "<link>https://weeklyosm.eu/archives/" | head -n 1 | cut -d "<" -f 2 | grep -o '[^/]*$')
else
article_id=$1
Copy link
Owner

Choose a reason for hiding this comment

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

we are calling our script with parameters - so I guess this would always go this path. $1 would be the first parameter, which is not the article id at the moment.

@call-me-matt
Copy link
Owner

initially, I was thinking that your proposed check would be better placed here.
But thinking about it, maybe not. I like how you determine the latest post id. Currently we call the script quite painfully with a list of parameters.
So maybe we can remove the check from the python script and use your script.

Please check it is compatible and remove the check from python, too.

And then, let's continue your idea and eliminate (in a second PR) the necessity to manually add the parameters:

--pic ~/Downloads/420_T_EN.jpg >> --pic downloads the lead picture if not followed by local path
420 (weekly issue number) >> could be determined automatically
10586 (weekly post id) >> is already determined by your script
2018-07-31 and 2018-08-06 >> can also be read automatically, I suppose

@@ -1,4 +1,6 @@
#!/bin/bash
set -e
Copy link
Owner

Choose a reason for hiding this comment

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

I am not a bash pro - but would that mean also the actual notification script would abort on error? For example if an error is thrown, because a telegram group removed permissions for our bot?

@Strubbl
Copy link
Contributor Author

Strubbl commented Mar 20, 2023

Currently we call the script quite painfully with a list of parameters.

ah, there is the list of languages

@call-me-matt
Copy link
Owner

your great idea to parse the parameters lead into this commit. Thank you for the inspiration!

@Strubbl
Copy link
Contributor Author

Strubbl commented Mar 23, 2023

Nice. If we already have the blog URL then, we can do the check for the favicon in python, too. So no need to update my bash script i would suggest

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