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

support ignoring file or directories #84

Merged
merged 2 commits into from
Aug 9, 2021
Merged

Conversation

willnorris
Copy link
Collaborator

use the doublestar library to support pattern matching of files or directories to ignore. This replaces (and deprecates) the previous -skip flag which only supported file extensions.

(previous discussion in #70 (comment))

The doublestar library requires go1.16, so I'm leaving this is draft mode until go1.17 is released (which should be any day now).

/cc @Shikugawa @laurentsimon @Shabirmean @lukehinds @cpanato - you've all been involved in similar implementations in #70, #73, and #75, so I'm curious if this implementation meets your use cases.

@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2021

Codecov Report

Merging #84 (b8695ac) into master (b431030) will increase coverage by 0.57%.
The diff coverage is 47.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #84      +/-   ##
==========================================
+ Coverage   46.98%   47.56%   +0.57%     
==========================================
  Files           2        2              
  Lines         232      246      +14     
==========================================
+ Hits          109      117       +8     
- Misses        116      122       +6     
  Partials        7        7              
Impacted Files Coverage Δ
main.go 40.19% <47.61%> (+1.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b431030...b8695ac. Read the comment docs.

Copy link

@Shabirmean Shabirmean left a comment

Choose a reason for hiding this comment

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

This is awesome. Thank you

main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

looks cool!

@naveensrinivasan
Copy link

What else is pending in the PR? This will be helpful to lots of other projects. Thanks

@willnorris
Copy link
Collaborator Author

I originally proposed we wait until the go1.17 release, so then we would be supporting the two latest releases of Go (since doublestar bumps our minimum version to go1.16). go1.17 should be releasing soon? RC2 was just released this past week.

We could choose to go ahead and merge and only support go1.16, and then try to support the latest two versions going forward once go1.17 drops. @mco-gh, what do you think?

@mco-gh
Copy link
Contributor

mco-gh commented Aug 6, 2021 via email

@naveensrinivasan
Copy link

I originally proposed we wait until the go1.17 release, so then we would be supporting the two latest releases of Go (since doublestar bumps our minimum version to go1.16). go1.17 should be releasing soon? RC2 was just released this past week.

We could choose to go ahead and merge and only support go1.16, and then try to support the latest two versions going forward once go1.17 drops. @mco-gh, what do you think?

Would appreciate it if we could merge this. Thanks

use the doublestar library to support pattern matching of files or
directories to ignore. This replaces (and deprecates) the previous
-skip flag which only supported file extensions.
@willnorris willnorris marked this pull request as ready for review August 9, 2021 18:49
@willnorris willnorris requested a review from mco-gh August 9, 2021 18:49
@willnorris
Copy link
Collaborator Author

okay, I've gone ahead and marked this ready for review @mco-gh. Since go1.16 has been out for 6 months, I suspect this is fine.

@naveensrinivasan
Copy link

Thank you!

@laurentsimon
Copy link

Awesome! Thanks for such a quick turnaround!

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.

7 participants