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

Log which file failed, clarify other files are still processed #908

Closed
wants to merge 1 commit into from

Conversation

raboof
Copy link

@raboof raboof commented Nov 22, 2018

No description provided.

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution! I am in favor of improving the console output so it can help users track down issues.

@@ -266,6 +266,8 @@ object MainOps {
i += 1
}
val next = handleFile(args, file)
if (next != ExitStatus.Ok)
args.args.out.println(s"Unexpected error handling '$file', proceeding to the next.")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid this message could be noisy and still not give enough information to track what happened. I think it is better to report the actual error at the root, do you have a reproduction the situation you encountered?

Copy link
Author

Choose a reason for hiding this comment

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

Sure: it happens when applying the 'Collection213CrossCompat' rules to akka. Enabling scalafix like this and running sbt 'akka-actor/scalafix Collection213CrossCompat' yields https://pastebin.com/NJ4N9xs9 (sorry about all the escape codes that get misrendered there).

This left me in the dark as to which file failed, and whether the scalafix process was aborted or continued after that. Of course having better errors nearer to the root would also be helpful, but perhaps not always practical.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for providing those details, very helpful.

I opened #910 to improve handling of unexpected errors so that the message includes what file is relevant.

@olafurpg
Copy link
Contributor

Superseded by #908, thank you @raboof for taking a stab at this!

@olafurpg olafurpg closed this Nov 28, 2018
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.

2 participants