-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/register created by and updated by #21
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks clean and great. You can try without request_store, take a look at my comment (disclaimer: I haven't used CurrentAttributes, seems like a new toy/double edge sword :P)
Forgot to say that is also worth to add updated_by, created_by to view page to be like updated_at, updated_by, created_at, created_by |
you mean the show page? I added it to the index. |
or you mean including the dates as well? |
Show page. |
I think it wasn't there. |
And we have annotate gem, so you can |
Done! I've left the dates out of the index table for now, as I'm not sure if it isn't too much. |
Yeah, dates should not be on index page, I agree. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 👍
This Pull Request adds: