perf(git): optimize git scan when exclude but no include filter is set #4823
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This can be a major optimization in certain cases. The reason this wasn't done previously was due to a git ls-files issue but we missed out on this path when no includes are used, in which case the issue doesn't come up. Usually includes are much more specific than excludes in configs, so this approach should cover a lot of user scenarios. It is still more optimal to use `.gardenignore` files over exclude patterns where possible, but using excludes in configs is obviously a common and supported use case, and not always possible to use the ignore files (say, if many actions/modules share a directory and need different exclusions).
|
I think this closes #4763? |
|
I believe so, yes. |
|
Does it also fix #4783? If yes it would be great if we add a test case that catches the issue described in #4783 (comment) to make this code more safe to change in the future 👍 |
|
It does not. Would be great to address that but it's a separate issue, and I wasn't able to repro it. That one might have something to do with submodules or at least some specific case that I'm not quite capturing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This can be a major optimization in certain cases.
The reason this wasn't done previously was due to a git ls-files issue but we missed out on this path when no includes are used, in which case the issue doesn't come up.
Usually includes are much more specific than excludes in configs, so this approach should cover a lot of user scenarios.
It is still more optimal to use
.gardenignorefiles over exclude patterns where possible, but using excludes in configs is obviously a common and supported use case, and not always possible to use the ignore files (say, if many actions/modules share a directory and need different exclusions).Fixes #4763