Skip to content
This repository has been archived by the owner on Dec 9, 2022. It is now read-only.

implement checking that dates are sufficiently recent #9

Closed
wants to merge 3 commits into from

Conversation

cherti
Copy link
Member

@cherti cherti commented Oct 25, 2015

this code checks whether an added talk is actually set in the future, if a date is supplied, to avoid placing talks in the past (be it intentionally or, more likely, by typo or mistake).

date, _ := time.ParseInLocation("2006-01-02", dateStr, loc)
date = date.Add(20*time.Hour)
if now.After(date) && date != locatedNullDate {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use date.IsZero() instead of locatedNullDate (also, that variable name is wrong, but that doesn't matter, as it's going away anyway).

Copy link
Contributor

Choose a reason for hiding this comment

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

Use time.Now().Sub(date) > 20*time.Hour instead of now and After and adding stuff to date.

@Merovius
Copy link
Contributor

This change seems counterproductive to me. We regularly edit talks after the fact (e.g. to add slides and recordings, the latter can be weeks after a talk), which seems impossible with this change and at least on one occasion, we had a completely new talk (an addition talk that was last-minute) that was added in the past for complete data. At the very least, this should a) check if it's a new talk to be added and allow it, if it's just an edit and b) be just a sanity-check prompt (i.e. "are you sure you want to do this?").

Maybe a little context could help. Was this actually a problem recently?

@cherti
Copy link
Member Author

cherti commented Oct 25, 2015

There is no problem currently, just thougth (inspired by the issue to just allow Thursdays) this might be an idea to have more sanity checking in that regard. However, I see your points that we might loose flexibility we actually want. I'll think about it if there is a reasonable way to address these points or if it's better left unmerged.

@Merovius
Copy link
Contributor

Merovius commented Nov 1, 2015

I'll close this for now. Feel free to reopen.

@Merovius Merovius closed this Nov 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants