-
-
Notifications
You must be signed in to change notification settings - Fork 308
added in-tree build artifacts to .gitignore #1823
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
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Git
Developer->>Git: Modify .gitignore
Git->>Git: Add new entries to ignore list
Git->>Repository: Update ignored files
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
afb0a78 to
cad9a0c
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.gitignore (1)
79-82: Adjust pattern consistency for build artifactsThe build artifact patterns should follow consistent rules:
- Remove leading slash from
Makefile.tmpas it could appear in subdirectories- Keep leading slashes for build tools (
makecrct,maketrees)- Consider if
gzread.cneeds a leading slash based on where it's generatedApply this diff:
-Makefile.tmp -/makecrct -/maketrees -gzread.c +Makefile.tmp +/makecrct +/maketrees +/gzread.cAdditionally, consider adding a comment to document that these are configure/make build artifacts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.gitignore(3 hunks)
🔇 Additional comments (2)
.gitignore (2)
67-67: LGTM!
The .cache directory entry follows the correct pattern style and is appropriately placed.
30-32: 🛠️ Refactor suggestion
Relocate and adjust style of header file entries
These header file entries should be moved near the other header patterns (around line 60) and the leading slashes should be removed to maintain consistency with the existing style.
Apply this diff:
-/zconf-ng.h.included
-/zlib_name_mangling-ng.h
-/zlib-ng.h
+zconf-ng.h.included
+zlib_name_mangling-ng.h
+zlib-ng.h| Makefile.tmp | ||
| /makecrct | ||
| /maketrees | ||
| gzread.c |
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.
💡 Codebase verification
Based on the build system analysis, I can now provide a final response:
Additional build artifacts need to be ignored
The project uses both CMake and Configure build systems. The current .gitignore is missing several common build artifacts:
- CMake build artifacts:
CMakeFiles/CMakeCache.txtcmake_install.cmakeCPackConfig.cmakeCPackSourceConfig.cmake*.a*.so**.dll*.dylib
- Build outputs:
minigzipminideflatelibz.*zlib.pc
- Test artifacts:
test/exampletest/minigziptest/gtest_zlib
These patterns should be added to ensure all build artifacts are properly ignored.
🔗 Analysis chain
Verify coverage of in-tree build artifacts
Let's ensure we haven't missed any common build artifacts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for typical configure/make artifacts in the repository
# that might not be covered by current patterns
# Look for configure-generated files
fd -H -E .git "config\.(log|status|cache)"
# Look for make-generated files
fd -H -E .git "\.deps$|\.libs$|\.dirstamp$"
# Look for common object file patterns not covered
fd -H -E .git "\.(la|lai|lo|o|so|so\.[0-9]+)$"
Length of output: 139
Script:
#!/bin/bash
# Let's check the build system and potential artifacts by:
# 1. Looking for build-related files to understand the build system
# 2. Checking for patterns of generated files in build instructions
# Check for build system files
fd -H -E .git "configure\.ac|configure\.in|Makefile\.am|Makefile\.in|CMakeLists\.txt|\.cmake$"
# Look at build instructions in documentation
if [ -f "README.md" ]; then
echo "=== README.md ==="
cat README.md | grep -A 5 -i "build\|compile\|make"
fi
if [ -f "INSTALL" ]; then
echo "=== INSTALL ==="
cat INSTALL | grep -A 5 -i "build\|compile\|make"
fi
# Check for any existing make targets
if [ -f "Makefile" ]; then
echo "=== Makefile targets ==="
grep "^[a-zA-Z0-9_-]*:" Makefile
fi
Length of output: 13362
Dead2
left a comment
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.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1823 +/- ##
============================================
+ Coverage 0 32.19% +32.19%
============================================
Files 0 67 +67
Lines 0 5752 +5752
Branches 0 1237 +1237
============================================
+ Hits 0 1852 +1852
- Misses 0 3644 +3644
- Partials 0 256 +256 ☔ View full report in Codecov by Sentry. |
when build with traditional
configure && makein-tree, some build artifacts will pop up that are not excluded by.gitignore.Summary by CodeRabbit
.gitignorefile to exclude additional files and directories, enhancing version control management.