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.

Sign up for GitHub

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

Jump to bottom

fix(injector): nil-dereference on a specific import order #315

Merged
merged 3 commits into from
Oct 2, 2024
Merged

fix(injector): nil-dereference on a specific import order #315

merged 3 commits into from
Oct 2, 2024

Conversation

eliottness
Copy link
Contributor

@eliottness eliottness commented Oct 2, 2024 •
edited
Loading

@eliottness eliottness requested a review from a team as a code owner October 2, 2024 13:43
RomainMuller
RomainMuller approved these changes Oct 2, 2024
View reviewed changes
internal/injector/injector.go
@@ -96,7 +96,7 @@ func (i *Injector) InjectFiles(files []string) (map[string]InjectedFile, error)

wg.Add(len(astFiles))
for idx, astFile := range astFiles {
go func(astFile *ast.File) {
go func(idx int, astFile *ast.File) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

internal/injector/write.go Outdated Show resolved Hide resolved
@eliottness
suggestion + fix header
65f123e 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness eliottness enabled auto-merge October 2, 2024 14:37
@eliottness eliottness added this pull request to the merge queue Oct 2, 2024
Merged via the queue into main with commit 124e6ab Oct 2, 2024
23 checks passed
@eliottness eliottness deleted the eliott.bouhana/fix-multiple-imports-segv branch October 2, 2024 15:11
@codecov Codecov
Copy link

codecov bot commented Oct 2, 2024

Codecov Report

Attention: Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Project coverage is 73.92%. Comparing base (b1593a1) to head (65f123e).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/injector/imports.go 92.85% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
- Coverage   73.99%   73.92%   -0.08%     
==========================================
  Files         145      146       +1     
  Lines        7811     7966     +155     
==========================================
+ Hits         5780     5889     +109     
- Misses       1596     1640      +44     
- Partials      435      437       +2
Components Coverage Δ
Generators 76.98% <ø> (ø)
Instruments 88.05% <ø> (ø)
Go Driver 72.81% <ø> (ø)
Toolexec Driver 70.88% <ø> (ø)
Aspects 70.88% <ø> (-0.99%) ⬇️
Injector 73.15% <94.44%> (-0.57%) ⬇️
Job Server 63.72% <ø> (ø)
Integration Test Suite 87.75% <ø> (-0.08%) ⬇️
Other 73.92% <94.44%> (-0.08%) ⬇️
Files with missing lines Coverage Δ
internal/injector/injector.go 73.52% <100.00%> (+1.22%) ⬆️
internal/injector/write.go 46.42% <100.00%> (+1.98%) ⬆️
internal/injector/imports.go 84.41% <92.85%> (-0.10%) ⬇️

... and 9 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RomainMuller RomainMuller RomainMuller approved these changes

@tonyredondo tonyredondo tonyredondo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] While instrumenting the rclone repo I got a SIGSEGV
3 participants
@eliottness @tonyredondo @RomainMuller