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

Fixed #908 -- Enable account notification for inviting friend to OH. #178

Merged
merged 1 commit into from Jan 28, 2014

Conversation

ArcTanSusan
Copy link
Contributor

Resolves https://openhatch.org/bugs/issue908. Feel free to code review and make suggestions for improvement.

New notification message after the Welcome to OH email has been sent to a friend (or enemy):

openhatch-issue 908-invite-somone

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f0fc3e7 on onceuponatimeforever:908 into 94beb63 on openhatch:master.

jwm added a commit that referenced this pull request Jan 28, 2014
Fixed #908 -- Enable account notification for inviting friend to OH.
@jwm jwm merged commit f41db40 into openhatch:master Jan 28, 2014
@jwm
Copy link
Contributor

jwm commented Jan 28, 2014

Merged - thanks, Susan!

I also made a couple small style improvements after merging your commit - you can see them at d35c2bc . In particular, .get() returns None by default, so it's nice to omit the default value, and I added a comma after the last dict item so future changes only have to change the line(s) they're adding or removing and don't need to worry about dealing with the comma.

@paulproteus
Copy link
Contributor

Hey @jwm and @onceuponatimeforever,

Thanks for writing & reviewing!

A meta-remark here: it seems like these bits of code style feedback could be automated, to decrease the latency with which contributors get the feedback.

One tool we could use for that is https://github.com/justinabrahms/imhotep ; we'd have to host it somewhere. Perhaps on vm3.openhatch.org, which is where our Jenkins is, or perhaps on Heroku.

I've filed https://openhatch.org/bugs/issue930 accordingly!

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

4 participants