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 technical event information #389

Merged

Conversation

johnsyweb
Copy link
Contributor

Context

I hit #385 and thought that I should have a go at fixing it rather than simply raising an issue. There's no obvious way to determine whether an event is "live" without scraping, which parkrun do not like - this change assumes all events listed are live.

Change

Remove all references to technical event information and the wiki domain.

Considerations

I like red diffs and I cannot lie.

@fraz3alpha
Copy link
Owner

@johnsyweb, why did you add an event_has_started() function, instead of assuming everything was live? Did something not work if you started ripping out the if statements/parts that depending on checking if it was live?

@johnsyweb
Copy link
Contributor Author

johnsyweb commented Jun 18, 2023

Good question. I didn't write the function, someone way smarter did. I simply refactored it to keep track of all the event_info.status == 'Live' || event_info.status == 'unknown' tests so that if I could find another way to determine whether an event was live, I could implement it once.

I don't think there's a way of doing this without hitting a whole bunch of pages (which is likely to result in firewall action), so this could probably be pulled out.

@johnsyweb
Copy link
Contributor Author

johnsyweb commented Jun 19, 2023

@fraz3alpha removing event_has_started() in e0cf2aa has no effect on the behaviour in either browser, so I think this is good to go.

@fraz3alpha fraz3alpha merged commit 097bda3 into fraz3alpha:master Jun 28, 2023
24 checks passed
@fraz3alpha
Copy link
Owner

All seems very reasonable, so I have merged it - thanks a lot!

@johnsyweb johnsyweb deleted the paj/rm-technical_event_information branch June 29, 2023 02:14
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.

None yet

2 participants