Skip to content

Fix displaying of new PEPs in python-news #2527

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

Closed
wookie184 opened this issue Apr 10, 2023 · 2 comments · Fixed by #2533
Closed

Fix displaying of new PEPs in python-news #2527

wookie184 opened this issue Apr 10, 2023 · 2 comments · Fixed by #2533
Assignees
Labels
t: bug Something isn't working

Comments

@wookie184
Copy link
Contributor

Currently we only send new PEP updates to #mailing-lists if they were "created" on the same day that the bot is checking for them.

if (
pep_nr in pep_numbers
or new_datetime.date() < date.today()
):
continue

This does not work in most cases, as when PEPs are created is not the same as when they are published.
https://peps.python.org/pep-0001/#pep-header-preamble

The Created header records the date that the PEP was assigned a number

The number is usually assigned quickly after the PR to https://github.com/python/peps is created, but then the PR is reviewed which means it can be a while before it's actually merged and becomes visible. The difference can be fairly big (e.g. python/peps#2620).

We can fix this by extending the check if it's in the last month (maybe last 2 months). The only issue I see with this is that in development people might get a few peps sent in a row when they first start up, but this seems pretty minor.

@wookie184 wookie184 added the t: bug Something isn't working label Apr 10, 2023
@mbaruh
Copy link
Member

mbaruh commented Apr 11, 2023

I'm somewhat struggling with the overall logic of that function. If we're keeping track of what PEPs are already shown in the channel, why do we need any date validation?

It also doesn't seem like there's an exit condition in the loop. Are we just going over the entire feed every time?

@wookie184
Copy link
Contributor Author

I'm somewhat struggling with the overall logic of that function. If we're keeping track of what PEPs are already shown in the channel, why do we need any date validation?

It shouldn't make a big difference in production, although in dev it prevents you being sent loads of posts when you first start up the bot. The feed only contains the past 10 PEPs so it would be fine without the validation, although a generous time check would also be fine imo.

It also doesn't seem like there's an exit condition in the loop. Are we just going over the entire feed every time?

Yeah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants