Alex Burkhart saterus

  • Neo Innovation
  • Columbus, OH
  • Joined on

Organizations

@edgecase @OSUOSC @DRSNJM @columbus-area-boardgaming-society
saterus created repository saterus/in-rust-we-trust
@saterus

@jdegoes I'd forgotten to submit the PR for this when I was originally added. I've rebased it to a recent commit.

@saterus
Add Alex Burkhart
1 commit with 35 additions and 0 deletions
@saterus
@saterus
@saterus
  • @saterus fc16ff9
    Update speaking engagements
@saterus
Handle OS signals to shutdown gracefully
1 commit with 15 additions and 0 deletions
@saterus

I'd usually try to remove the logging statements before committing, but the second best thing is what you just did. :+1:

@saterus

Why remove name from the form? Now they can't edit their name, since they can only input it on the sign up page.

@saterus

I would normally recommend finding a specific User object first, then calling destroy on it alone. This does work, but definitely not preferred. de…

@saterus

:+1: remembering to sign them out before blowing up their account. Otherwise they get stuck in an error loop.

@saterus

This is okay, since you can't actually send a DELETE request without js unless you use a form. Ideally you end up doing this with js and it is just…

@saterus

Yeah, I don't think these are what you want. Even if this is working, you should be using regular anchor tags.

@saterus

Why did you make a link into a form? Where did you find the LINK method? I don't believe the docs you were following here were the right ones...

@saterus

Great commit. :100:

@saterus

Not on <a>s anyway.

@saterus

title isn't a real attribute.

@saterus

:+1: Better place for the error handling to live. Now we can start talking about using CSS for styling this error message...

@saterus

Looks like you are missing some error handling here. Failed updates are silently swallowed.

@saterus

:+1: Makes it much easier to know if login worked.

@saterus

:+1: to the comments. Also good job remembering to assign to @current_user in this helper. No need to go back to the database to fetch a User we just …

@saterus

See my comment on current_user about naming this more explicitly.

@saterus

A whole lot of nitpicking here: I tend to prefer .nil? instead of == nil. It is just as intention revealing, but I can't easily typo it and assign nil

@saterus

Great to see you using locals instead of just accessing the instance variables inside the partials. It becomes really hard to trace back errors onc…

@saterus

I love that you're adding validation errors to the page. Probably a good candidate for DRYing up into a partial, since this looks very similar to t…

@saterus

I like you proactively adding seed data. Keeps you honest, as your screens will have some data on them.

@saterus

Once again, you shouldn't need to try and return this value on another line.