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 the name of the file that is not processed by SUSHI #1307

Merged
merged 4 commits into from
Jul 24, 2023

Conversation

mrinnetmaki
Copy link
Contributor

Add a log message for each of the files not processed by SUSHI.

I get that the current implementation is faster, and also results in fewer log messages when there are a lot of files in the input directory that are not processed by SUSHI. But there should not be that many such files, right? So let's just print the name of each of the files, and then the total count too. It's much more helpful.

Fixes #1306.

@cmoesel
Copy link
Member

cmoesel commented Jul 19, 2023

Thanks for submitting a PR, @mrinnetmaki. It looks like it failed our file formatting check. The easiest way to fix this is to run: npm run prettier:fix.

You can check that all the files follow our formatting rules (and that all tests pass) by running: npm run check.

As for the functionality, I worry a little about issuing lots of extra info messages for people who might have many non-xml/non-json files in their input (for whatever reason). How would you feel about changing this in one of the following ways:

  • Change the logger to log the extra files using debug logging -- and in the message that tells you there were non-json/non-xml files, add this at the end: To see the unprocessed files in the logs, run SUSHI with the "--log-level debug" flag.
  • OR... Instead of logging each file as a separate info message, collect the file paths in an array and then append them to the end of the message that says there were non-xml/non-json files.

How would you feel about either of those changes?

@mrinnetmaki
Copy link
Contributor Author

Thanks for the feedback, @cmoesel! Using debug level log message is OK.

Copy link
Member

@cmoesel cmoesel 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, @mrinnetmaki. This looks good to me!

Copy link
Collaborator

@jafeltra jafeltra left a comment

Choose a reason for hiding this comment

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

This looks like it will be helpful. I like the debug message approach. Thanks!

@jafeltra jafeltra merged commit 8dc0129 into FHIR:master Jul 24, 2023
7 checks passed
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.

Log the path and name of files that are not processed
3 participants