Skip to content

Support find .sarif files recursively #352

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

Merged
merged 1 commit into from
Jan 4, 2021
Merged

Conversation

chao2zhang
Copy link
Contributor

This addresses #220 by recursively calling fs.readdirSync()

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this. The implementation is nice and simple and looks good to me.

The only thing I was worried about was if we'd get into loops with symbolic links, but looking at the documentation it looks like it'll be fine, and I even gave it a try locally and it was fine.

I think we could add a unit test of this but rather than delaying this PR it'll probably be easiest to merge now and I'll add a quick test.

@jwgmeligmeyling
Copy link

The workings of this feature is not documented in the action.yml nor readme, despite the checkmark for it.

Some examples would be helpful. For example, this recursive implementation does only work on folder names and not file names.

@aeisenberg
Copy link
Contributor

Some examples would be helpful. For example, this recursive implementation does only work on folder names and not file names.

Can you be more specific? You can specify a directory (which gets searched recursively for sarif files or a single file. Is there a feature you are looking for, but is not implemented here? Or are you just seeking clarity on the behaviour?

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.

4 participants