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

Code style refactor #1546

Merged
merged 1 commit into from Aug 22, 2012
Merged

Code style refactor #1546

merged 1 commit into from Aug 22, 2012

Conversation

accessd
Copy link
Contributor

@accessd accessd commented Jul 22, 2012

No description provided.

@travisbot
Copy link

This pull request passes (merged fc1578d6 into 5c4e75a).

@@ -5,7 +5,7 @@ def initialize(html, table_css_selector = "table")
end

def to_array
rows = Nokogiri::HTML(@html).css("#{@selector} tr")
rows = Nokogiri::HTML(@html).css("#@selector tr")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not many people know the conditions under which braces can be omitted. It's clearer to leave the {} in. Most projects do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Agree.

@travisbot
Copy link

This pull request passes (merged 65fe1aa into c0a5d51).

@@ -17,7 +17,7 @@ def filter(method, options = {})

# Returns the default filter type for a given attribute
def default_input_type(method, options = {})
if column = column_for(method)
if (column = column_for(method))
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't usually do this. I guess it's a matter of taste? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe :) Everyone follows his style. But Github recommends doing so https://github.com/styleguide/ruby/. In this particular case may be obvious whether a value is assigned to a variable or two variables are compared. You decide.

Copy link
Contributor

Choose a reason for hiding this comment

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

GitHub FTW!

pcreux added a commit that referenced this pull request Aug 22, 2012
@pcreux pcreux merged commit ce6f3fd into activeadmin:master Aug 22, 2012
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