Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Added AST dumping. #193

Merged
merged 3 commits into from
Jul 20, 2017
Merged

Added AST dumping. #193

merged 3 commits into from
Jul 20, 2017

Conversation

zenspider
Copy link
Contributor

Add ast_dump:true to your .codeclimate.yml file's config.

Running codeclimate will now output the AST to stderr, allowing you to
figure out filter patterns for issues you don't care about.

See README.md for more details.

Add ast_dump:true to your .codeclimate.yml file's config.

Running codeclimate will now output the AST to stderr, allowing you to
figure out filter patterns for issues you don't care about.

See README.md for more details.
Copy link
Contributor

@gdiggs gdiggs left a comment

Choose a reason for hiding this comment

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

Just a few minor style changes - this looks great! The documentation is really good as well

@@ -93,8 +133,13 @@ def flay_options
CCFlay.default_options.merge changes
end

def debug(message)
$stderr.puts(message) if engine_config.debug?
require "thread"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this require to the top of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. It was actually already required (tho not sure why?). I also moved the constant up for clarity.

def debug(message)
$stderr.puts(message) if engine_config.debug?
require "thread"
IO_M = Mutex.new
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this constant declaration to the top of the class above def initialize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


process_files

return dump_ast if engine_config.dump_ast?
Copy link
Contributor

Choose a reason for hiding this comment

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

💄 small formatting request:

if engine_config.dump_ast?
  dump_ast
else
  report
  debug("Reported #{reports.size} violations...")
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah! I picked up that style from rubocop. Sure. I can change it to something more conventional.

@zenspider
Copy link
Contributor Author

This is up to date w/ the review now.

Removes filter extension from CCFlay.
Copy link
Contributor

@gdiggs gdiggs left a comment

Choose a reason for hiding this comment

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

LGTM once green

@gdiggs gdiggs merged commit e1802c2 into codeclimate:channel/beta Jul 20, 2017
gdiggs pushed a commit that referenced this pull request Jul 21, 2017
* Added AST dumping.

Add ast_dump:true to your .codeclimate.yml file's config.

Running codeclimate will now output the AST to stderr, allowing you to
figure out filter patterns for issues you don't care about.

See README.md for more details.

* Style changes per review.

* Bumped to flay 2.10+ and ruby_parser 3.10+.

Removes filter extension from CCFlay.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants