Provide file names when "Failed to process {len(self.discovery.failures)} files" is returned

ryancheley commented 3 years ago

When running tryceratops if any of the files aren't able to process, more information should be returned about which files can't be processed.

Specifically, running tryceratops on a simple project with only 16 files returns the following:

Done processing! 🦖✨
Processed 16 files
Found 0 violations
Failed to process 1 files
Skipped 2340 files

Without knowing which file failed to process, I'm not sure what, if anything, I should do for next steps.

This message comes from interfaces.py line 51

guilatrova commented 3 years ago

Good point. I was thinking about adding a -vv verbose flag to display all files. What do you think?

ryancheley commented 3 years ago

I think that's a great idea! If you'd like I can try and make and update and submit a PR. It would be a couple of days though before I was able to get the PR submitted.

ryancheley commented 3 years ago

@guilatrova I tried to get the project set up and installed locally in edit mode (pip install -e .) but I get the following errors:

ERROR: File "setup.py" not found. Directory cannot be installed in editable mode: /Users/ryan/github/tryceratops
(A "pyproject.toml" file was found, but editable mode currently requires a setup.py based build.)

What is the set up you use when developing locally. Perhaps I'm just missing something obvious.

Thanks!

guilatrova commented 3 years ago

@ryancheley Thanks for the help! I created a short new contributing doc, can you check and let me know if it works for you?

ryancheley commented 3 years ago

@guilatrova I've made the changes and pushed them to my fork of the repo. Before I create a pull request, do you want to look at the changes I made? If so, you can see them here

If not, just let me know and I'll create the PR

Thanks!

guilatrova commented 3 years ago

That's a great start and thanks for following the conventions!

I think we can go even beyond though! I'd love to make your change so impactful that it can benefit other developers/contributors in the future.

How? Here:

Verbose means that any "verbose" (duh) message gets displayed in the console, besides the final user presentation (that we do with print), so for any developer today and in the future that wants to benefit from the flag you just implemented they should be able to do this ANYWHERE in Tryceratops and it should work.

I'd recommend then:

Extend `logging` for that

__main__.py -> Once you receive the flag, set the log level to DEBUG Reference
settings.py -> Create a new handler to output to console - what if we name this handle verbose_output? Feel free to name it as you prefer
settings.py -> Create a better format for the new DEBUG handler (Maybe lineno and asctime is too much. Try and share your decision!)

Do not use prints, do not bloat the presentation layer

interfaces.py -> Remove the prints/ifs from the presentation layer
discovery.py -> Use logging.debug inside the except block (should we remove the exception one? wdyt?)

Congratulations

🎉🥳 Now ANY DEV can use logging.debug ANYWHERE and it WORKS!

Overall

Does it make sense? I'm curious to receive your feedback. I'm not completely sure if it would work though, but I believe it worths trying.

I can help with if you're too busy, but I'd love to have your name as a contributor in this repository ❤️

ryancheley commented 3 years ago

I really like the ideas above and can work on implementing something this week.

I'll need to look more closely at the try / except in discovery.py but based on my initial (very cursory) glance, I'd say it should be kept.

It seems like it's doing exactly what a try/except should be doing and I'm not aware of any other mechanisms to handle what it's doing (though maybe there is something?)

I'll continue to work on the branch of my fork and won't submit a PR for now.

Thanks!

P.S. I'd also love to have my name as a contributor on this project 😄

ryancheley commented 3 years ago

@guilatrova I think I have it figured out (also, I understand what you were asking about when you mentioned exception in your comment / question above) 🤦

I did swap out the logger.exception with logger.debug in discovery.py

My changes are located here.

There are still tests that need to be written for this change, but I wanted to get your input before starting to write the tests

Thanks!

guilatrova commented 3 years ago

Yes, you got the idea! Please open a PR so we can start discussing there ☺️

guilatrova / tryceratops