Skip to content

Conversation

@pmckernin
Copy link

@pmckernin pmckernin commented Dec 3, 2020

I have removed the extra div's leftover from bootstrap col in the index and show view.

I also changed the wording of the comment next to skip_before_action and force_signin to lead you to the other place.

This gitpod workspace has the generator branch changed and has already created an account and a resource.

Please feel free to create more resources and check to see if they work.

Open in Gitpod

# Uncomment this if you want to force #{plural_table_name} to sign in before any other actions
# Uncomment the "before_action" if you want to force #{plural_table_name} to sign in before any other actions
# Remember to also uncomment the "skip_before_action" in the corresponding authentication controller
Copy link
Contributor

@raghubetina raghubetina Dec 3, 2020

Choose a reason for hiding this comment

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

Perhaps the comment could be made even more direct with something like in <%= class_name.singularize %>AuthenticationController" rather than "in the corresponding authentication controller"?

@raghubetina
Copy link
Contributor

raghubetina commented Dec 3, 2020

Looks good. Made one suggestion.

Tiny nitpick for future: consider separating different tasks (e.g. the auth comments vs the divs) into different commits/PRs. This is to make it easier to review and also to make it easier to revert a commit later if e.g. a bug is discovered without losing other unrelated work.

Don't split this one up though; just keep it in the back of your mind.

@jelaniwoods
Copy link
Contributor

@pmckernin Rewording the comments makes sense to make instructions more explicit for the students, but what was the motivation behind removing the divs?

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.

4 participants