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

Change directory with a block while running the reporter #41

Merged
merged 1 commit into from
Nov 11, 2015

Conversation

dblandin
Copy link
Contributor

I experienced some weirdness recently with the error described in #40 showing up multiple times in the container logs. Using the block form of chdir seems to have removed that repetition, with just one expected occurrence showing up. I wonder if there's some strangess around changing directories and multiple jruby threads that I still don't understand, but using the block form seems to be a safe move.

Thoughts?

@@ -31,13 +31,16 @@ def run
languages_to_analyze.each do |language|
engine = LANGUAGES[language].new(engine_config: engine_config)
reporter = CC::Engine::Analyzers::Reporter.new(engine_config, engine, io)
reporter.run

Dir.chdir(directory) do
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it work to wrap the entire Duplication#run body in the chdir? That is closer to what we do in other repos and also closer to the original intention of doing it in the constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll make that change. 👍

@dblandin dblandin force-pushed the devon/chdir-into-directory-for-each-engine branch from 6d03a99 to 43bfc9e Compare November 11, 2015 15:53
@gdiggs
Copy link
Contributor

gdiggs commented Nov 11, 2015

👍

dblandin added a commit that referenced this pull request Nov 11, 2015
…r-each-engine

Change directory with a block while running the reporter
@dblandin dblandin merged commit f5da437 into master Nov 11, 2015
@dblandin dblandin deleted the devon/chdir-into-directory-for-each-engine branch November 11, 2015 16:01
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.

2 participants