-
Notifications
You must be signed in to change notification settings - Fork 33
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
Exclude .git directory content when calculating package file count #525
Conversation
@mpcen ready for review |
26df2bf
to
6eead81
Compare
@jeffwilcox @mpcen rebased and ready for review. |
@elrayle ready for review |
The existing code only exclude .git directory itself, but not its containing files. This fix intends to exclude .git directory and its contents. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like where you are going with this, especially pulling out the validation into a separate function. I have a question about the expectation of the validation and whether or not the use of intersection could have false negatives.
ea3c808
to
ca3901a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see this bug fixed. Really nice to see a function created that can be called from multiple places resulting in the same behavior.
@@ -150,7 +150,7 @@ class AbstractProcessor extends BaseHandler { | |||
*/ | |||
async filterFiles(location) { | |||
const fullList = await this.getFiles(location) | |||
const filteredList = fullList.filter(file => isValidExcludingGit(file)) | |||
const filteredList = fullList.filter(file => file && !isGitFile(file)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't necessarily need to check if file is undefined
here since isGitFile
already does: if (!file) return false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... I am afraid that the check for undefined file is still necessary. In case of falsy file, isGitFile(file) returns false, and !isGitFile returns true. Using !isGitFile alone would not exclude falsy file values.
This is for fix to exclude .git directory content in recent PR (clearlydefined#525). Bump up the version to allow reharvest of pod components.
The recent fix to exclude content in the .git directory (clearlydefined#525) from pod packages will cause the file count to be different from the previous version. Update the toolVersion for PodExtract to 2.0.0 to reflect this.
This was discovered during sanity test for upgrading crawler image to use node:18-bullseye. The file count for pod/cocoapods/-/SoftButton/0.1.0 was different:
The content of .git directory should be excluded for the package file count.
Task: #529