-
Notifications
You must be signed in to change notification settings - Fork 2
allow multiple hashtags #13
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,10 +108,10 @@ def get_parser(): | |
| ) | ||
|
|
||
| update.add_argument( | ||
| "--hashtag", | ||
| dest="hashtag", | ||
| "--hashtags", | ||
| dest="hashtags", | ||
| default="#RSEng", | ||
| help="A hashtag (starting with #) to include in the post, defaults to #RSEng", | ||
| help="A comma separated list of hashtags (starting with #) to include in the post, defaults to #RSEng", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For args that are intended to become lists, you can use hashtags = args.hashtags or []To ensure you get an empty list. The default can also be find-updates.py --hashtag first --hashtag secondSo I would keep the argument dest as just
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good idea. I like this method better. I'll make that change soon.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I've partially addressed this by returning the command-line argument to its original name. It can still accept a comma-separated list of hashtags though. I considered the suggestion to supply it as a list in the first place, but there are two reasons that I'd rather not do that and just keep it as a string:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the default, what you'd want to do is set to args.hashtag = args.hashtag or ["#RSEng"]Agree on the second point.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure I understand what you mean, but I'd rather not do it this way if it's OK with you. Are you still willing to merge it without this change? If so, I'll run a few more tests and point you to the results.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh no, I'm good with what you have - I'm just showing for the future how I'd handle the issue you pointed out. |
||
| ) | ||
|
|
||
| update.add_argument( | ||
|
|
@@ -282,7 +282,7 @@ def deploy_slack(webhook, message): | |
| ) | ||
|
|
||
|
|
||
| def deploy_bluesky(client, entry, keys, hashtag): | ||
| def deploy_bluesky(client, entry, keys, hashtags): | ||
| """ | ||
| Deploy to bluesky. We add the job link separately. | ||
| """ | ||
|
|
@@ -291,11 +291,12 @@ def deploy_bluesky(client, entry, keys, hashtag): | |
| # Prepare the post, but without the url | ||
| post = prepare_post(entry, keys, without_url=True) | ||
| choice = random.choice(icons) | ||
| message = f"New {hashtag} Job! {choice}\n" | ||
| print(message) | ||
|
|
||
| # Add the text to the textbuilder | ||
| tb.text(message) | ||
| # seems like a clumsy way to build such a message... | ||
| tb.text("New") | ||
| for hashtag in hashtags: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can do list comprehension instead. And I think tb.text() is going to need to be the entire message? I'm not sure you need "New" that is probably from an example. I would remove that, then do: New #one #two #three Job! 💼You can technically combine the two cleanup and add back into one comprehension, but I'd choose this one because it's easier to read.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I did a slightly cleaner replacement in |
||
| tb.text(" ").tag(hashtag, hashtag.strip("#")).text(" ") | ||
| tb.text(f"Job! {choice}\n") | ||
| anchor=entry.get('title') or entry.get('name') or entry.get('url') | ||
| tb.link(anchor, entry.get('url')) | ||
| tb.text("\n" + post) | ||
|
|
@@ -370,6 +371,9 @@ def help(return_code=0): | |
| # Parse keys into list | ||
| keys = [x for x in args.keys.split(",") if x] | ||
|
|
||
| # Parse hashtags into list | ||
| hashtags = [x for x in args.hashtags.split(",") if x] | ||
|
|
||
| # Find new posts in updated | ||
| previous = set() | ||
| missing_count = 0 | ||
|
|
@@ -410,8 +414,9 @@ def help(return_code=0): | |
| # Prepare the post | ||
| post = prepare_post(entry, keys) | ||
| choice = random.choice(icons) | ||
| message = f"New {args.hashtag} Job! {choice}: {post}" | ||
| newline_message = f"New {args.hashtag} Job! {choice}\n{post}" | ||
| tags=" ".join(x for x in hashtags) | ||
|
crd477 marked this conversation as resolved.
Outdated
|
||
| message = f"New {tags} Job! {choice}: {post}" | ||
| newline_message = f"New {tags} Job! {choice}\n{post}" | ||
| print(message) | ||
|
|
||
| # Convert dates, etc. back to string | ||
|
|
@@ -435,7 +440,7 @@ def help(return_code=0): | |
| deploy_twitter(twitter_client, newline_message) | ||
|
|
||
| if args.deploy_bluesky and bluesky_client: | ||
| deploy_bluesky(bluesky_client, entry, keys, args.hashtag) | ||
| deploy_bluesky(bluesky_client, entry, keys, hashtags) | ||
|
|
||
| # If we are instructed to deploy to mastodon and have a client | ||
| if args.deploy_mastodon and mastodon_client: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.