[kernel-869] exclude files from extensions #98
Conversation
archandatta
force-pushed
the
archand/kernel-869/exclude-files-from-extensions
branch
from
a9498e8 to
cf9bfed
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
thanks for the pr! left some comments on naming, code duplication, and making the util more generic. main concerns are around duplicating the existing ZipDirectory function and making the exclusion options more intuitive.
|
|
||
| const ( | ||
| // MaxExtensionSize is the maximum allowed size for extension bundles from API (50MB) | ||
| MaxExtensionSize = 50 * 1024 * 1024 |
There was a problem hiding this comment.
nit: consider renaming to MaxExtensionSizeBytes to be more explicit about units
| pterm.Success.Printf("Created bundle: %s (%d files)\n", | ||
| util.FormatBytes(fileInfo.Size()), stats.FilesIncluded) | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: obvious comment, can remove
| return fmt.Errorf("bundle exceeds maximum size") | ||
| } | ||
|
|
||
| // Open file for upload |
There was a problem hiding this comment.
nit: obvious comment, can remove
| if err != nil { | ||
| return fmt.Errorf("failed to open temp zip: %w", err) | ||
| } | ||
| defer f.Close() |
There was a problem hiding this comment.
nit: obvious comment, can remove
| return util.CleanedUpSdkError{Err: err} | ||
| } | ||
|
|
||
| // Display results |
There was a problem hiding this comment.
nit: obvious comment, can remove
|
|
||
| func (s *ZipStats) AddIncluded(bytes int64) { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() |
There was a problem hiding this comment.
verify mutex usage is correct here - should be using s.mu.Unlock() not s.mutex.Unlock()
|
|
||
| // ZipExtensionDirectory zips a Chrome extension directory with smart defaults | ||
| // that automatically exclude development files (node_modules, .git, etc.) | ||
| func ZipExtensionDirectory(srcDir, destZip string, opts *ExtensionZipOptions) (*ZipStats, error) { |
There was a problem hiding this comment.
there's already a ZipDirectory function in pkg/util/zip.go that does almost exactly the same thing (used by deploy and browsers commands). this duplicates a lot of code. consider enhancing the existing ZipDirectory with options and stats support rather than creating a new function. also note that the existing ZipDirectory has a potential goroutine leak (doesn't call walker.Terminate()).
| t.Errorf("Expected file not found in zip: %s", name) | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
verify the closing brace for the first t.Run is present - the diff showed a potential missing brace here
|
|
||
| expectedPatterns := []string{"*.test.js", "*.test.ts", "*.spec.js", "*.spec.ts", "*.log", "*.swp"} | ||
| for _, pattern := range expectedPatterns { | ||
| if !contains(DefaultExtensionExclusions.ExcludeFilenamePatterns, pattern) { |
There was a problem hiding this comment.
verify there's no duplicate if keyword here (should be if !contains(...) not if if !contains(...))
|
|
||
| // Helper function to check if a slice contains a string | ||
| func contains(slice []string, str string) bool { | ||
| for _, s := range slice { |
There was a problem hiding this comment.
verify there's no duplicate range keyword here (should be for _, s := range slice not for _, s := range range slice)
Note
Introduces an extension-specific zipping flow with sensible defaults and size guards to streamline uploads.
pkg/util/zip_extensions.go: addsZipExtensionDirectorywith default exclusions (node_modules,.git,__tests__,coverage, test/log/temp file patterns), symlink handling, andZipStats. Comprehensive tests inpkg/util/zip_extensions_test.go.cmd/extensions.goupload path now usesZipExtensionDirectory, shows progress, prints bundle summary (FormatBytes, file count), and enforcesMaxExtensionSize(50MB) with reduction suggestions before uploading.util.FormatBytesinpkg/util/format.gofor human-readable sizes.Written by Cursor Bugbot for commit cf82964. This will update automatically on new commits. Configure here.