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

Clean up rubocop offenses #522

Merged
merged 10 commits into from
Feb 21, 2019
Merged

Conversation

simmerz
Copy link
Contributor

@simmerz simmerz commented Feb 19, 2019

This PR cleans up various rubocop offenses

I've also regenerated the .rubocop-todo.yml so it's up to date and doesn't contain things that have been fixed by cleaning up code elsewhere.

@simmerz simmerz changed the title Clean up Metrics/AbcSize offences Clean up Metrics/AbcSize and Metrics/MethodLength offences Feb 19, 2019
@simmerz simmerz changed the title Clean up Metrics/AbcSize and Metrics/MethodLength offences Clean up rubocop offenses Feb 19, 2019
@simmerz
Copy link
Contributor Author

simmerz commented Feb 19, 2019

@lcreid I've done a load of work refactoring here and am now left with 2 files that I think should be in test/ - bootstrap_deferred_builder_test.rb and bootstrap_error_rendering_test.rb

Is there any reason those are in the root of the project?

@lcreid
Copy link
Contributor

lcreid commented Feb 19, 2019

Ouch! Those were test files that I was using locally at some point. I obviously should have moved them out of the tree completely, because somewhere along the line I committed them by accident. Sorry. Feel free to remove them. Or let me deal with it later.

@simmerz
Copy link
Contributor Author

simmerz commented Feb 19, 2019

Ah ok. I did wonder seeing as I couldn't see how they could possibly pass! I'll remove them in a bit and then we're done with the to-do file.

@simmerz
Copy link
Contributor Author

simmerz commented Feb 20, 2019

@lcreid all done

Copy link
Contributor

@lcreid lcreid left a comment

Choose a reason for hiding this comment

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

Awesome work! This code is so much easier to read. It's a pleasure to read it now. I especially like what you've done for check boxes and radio buttons. That was hard code to figure out.

The only changes I think we need to do are the ones around what we declare to be HTML safe. You'll see the detailed comments below. If making the changes I asked for breaks a test case, then we need to discuss. As always, please push back if you think I'm asking for the wrong thing, or if I'm missing something.

demo/app/helpers/bootstrap_helper.rb Show resolved Hide resolved
lib/bootstrap_form/helpers/bootstrap.rb Show resolved Hide resolved
lib/bootstrap_form/helpers/bootstrap.rb Outdated Show resolved Hide resolved
lib/bootstrap_form/helpers/bootstrap.rb Outdated Show resolved Hide resolved
lib/bootstrap_form/helpers/bootstrap.rb Outdated Show resolved Hide resolved
lib/bootstrap_form/inputs/file_field.rb Outdated Show resolved Hide resolved
lib/bootstrap_form/inputs/file_field.rb Outdated Show resolved Hide resolved
lib/bootstrap_form/inputs/check_box.rb Show resolved Hide resolved
@simmerz
Copy link
Contributor Author

simmerz commented Feb 21, 2019

@lcreid Updates pushed. Thanks for the review!

@lcreid lcreid merged commit aa2cb29 into bootstrap-ruby:master Feb 21, 2019
@lcreid
Copy link
Contributor

lcreid commented Feb 21, 2019

Once again, a big thanks for all the work you've done on this gem!

@simmerz simmerz deleted the rubocop-abc_size branch February 21, 2019 16:55
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

2 participants