Skip to content

Conversation

@parkan
Copy link
Collaborator

@parkan parkan commented Mar 25, 2025

fixes for linter env and lint errors

@parkan parkan changed the title [DRAFT] Chore/linter [DRAFT] chore/linter Mar 25, 2025
@parkan parkan changed the title [DRAFT] chore/linter [WIP] chore/linter Mar 25, 2025
@parkan parkan mentioned this pull request Apr 30, 2025
@parkan
Copy link
Collaborator Author

parkan commented Apr 30, 2025

side note, I felt bad about breaking the build/release previously but apparently half of the recent https://github.com/golangci/golangci-lint/releases are broken/didn't have correct assets published so I'm in good company 😅

@parkan
Copy link
Collaborator Author

parkan commented May 1, 2025

well, that took... quite a bit longer than ideal but we have a 🟢 build

notes:

  • go version is now inferred from go.mod instead of specified in a bunch of places, this may be causing "go next" builds to not actually use go next, I need to review that but not having a single source of truth is 🤯
  • a lot of linters have changed their behavior even with the same name and settings, we do our best to work with this
  • suppressing the integer conversion overflow lint errors is not ideal but we are so freewheling with it in the codebase that I ended up throwing in the towel (aside from a few places where checks have been added, primarily file sizes) -- I did review each case flagged by the linter so far and concluded that under reasonable circumstances assumptions like "number of CAR flles or ranges are non-negative" will hold, however this is by no means suitable for use in adversarial environments
  • I've tried to strike a balance between addressing legitimate concerns and not diverging too much from existing behaviors, the most significant change is that we're now always passing Server as pointer -- this may need a second set of eyes to find corner cases but I generally believe this to be correct and good
  • github actions/workflows are the most cursed ecosystem

@parkan parkan changed the title [WIP] chore/linter chore/linter May 1, 2025
@parkan parkan marked this pull request as ready for review May 1, 2025 15:07
@parkan parkan requested a review from Sankara-Jefferson May 1, 2025 15:30
@zachfedor
Copy link
Collaborator

This is great! I was trying to follow the CONTRIBUTING.md guidelines but the make lint command was giving so much unrelated output, I was wondering if it was still being used. I appreciate the effort!

@which golangci-lint > /dev/null || (echo "Required golangci-lint not found. Installing it..." && GO111MODULE=on go get github.com/golangci/golangci-lint/cmd/golangci-lint@latest)
@which staticcheck > /dev/null || (echo "Required staticcheck not found. Installing it..." && GO111MODULE=on go get honnef.co/go/tools/cmd/staticcheck)
@which golangci-lint > /dev/null || (echo "Required golangci-lint not found. Installing it..." && go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest)
@which staticcheck > /dev/null || (echo "Required staticcheck not found. Installing it..." && go install honnef.co/go/tools/cmd/staticcheck@latest)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we might want to lock this but keeping the local version on latest does make it a bit more likely that we'll keep up with upstream changes

@Sankara-Jefferson Sankara-Jefferson merged commit 151fae5 into main May 14, 2025
11 checks passed
@Sankara-Jefferson Sankara-Jefferson deleted the chore/linter branch May 14, 2025 03:50
Sankara-Jefferson pushed a commit that referenced this pull request Jun 14, 2025
fixes for linter env and lint errors

---------

Co-authored-by: Arkadiy Kukarkin <arkadiy@archive.org>
anjor pushed a commit to anjor/singularity that referenced this pull request Jun 14, 2025
fixes for linter env and lint errors

---------

Co-authored-by: Arkadiy Kukarkin <arkadiy@archive.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants