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

don't allow scheduling for non-thursdays #10

Merged
merged 1 commit into from
Oct 28, 2015
Merged

Conversation

cherti
Copy link
Member

@cherti cherti commented Oct 27, 2015

Don't allow scheduling a talk on a non-thursday. Solves issue #8.

@@ -223,6 +223,11 @@ func handlePost(res http.ResponseWriter, req *http.Request) {

date, _ := time.ParseInLocation("2006-01-02", dateStr, loc)

if date.Weekday() != 4 && !time.IsZero() {
Copy link
Contributor

Choose a reason for hiding this comment

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

s/4/time.Thursday/
s/time/date/ (that should have popped up when trying to build)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, while you're on it, switch the conditionals around. First the validity-check, then the specific check :) And, strictly speaking, date.IsZero() would imply date.Weekday() != 4, but I think it's still good to have both checks for readability.

Copy link
Member Author

Choose a reason for hiding this comment

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

we cannot neglect the date.Weekday()-check, because there might be non-zero dates that are not thursdays (e.g. due to typo in the date).* One could do two ifs, though, but I'd prefer the one-if-version for readability.

*) Or did I misunderstood what you ment?

@Merovius
Copy link
Contributor

I would prefer to squash these commits (as they are quite small and don't build anyway) and put a "Fixes #8" into the commit-message (in a separate) line.

@Merovius
Copy link
Contributor

If you are squashing commits, rebase on master too, while you're at it. I enabled travis builds in the meantime to catch build-failures automatically and it'd be good to include that just for completenes :)

@cherti
Copy link
Member Author

cherti commented Oct 28, 2015

done, should be ready for merge now.

Merovius added a commit that referenced this pull request Oct 28, 2015
don't allow scheduling for non-thursdays
@Merovius Merovius merged commit 1b1bfe0 into master Oct 28, 2015
@Merovius Merovius deleted the nononthursdays branch October 28, 2015 17:26
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