-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
Support for module plugin system via .custom-gcl.yml #1076
Comments
Hello, We declined The suggestion of the
So this suggestion should not be followed if you don't understand the limitations of this appraoch. |
Hi! Thanks for the perspective.
I completely understand why it was not accepted as a golangci-lint linter, but I'm actually ok with adding a few Keeping nilaway apart, I'm thinking if the module plugin system exists in golangci-lint, it would be a good thing to integrate it with the GH action anyway. I'm currently looking in custom company linters so the plugin support would come handy (and I like the convenience of golangci-lint to run even custom linters)
I'd say this can be fixed by encouraging people to use a tagged version in the documentation. Maybe a warning could be issued by the GH action if it detects a non-semantic versioning tag (or a non numeric tag) in the The
I think adding the hash of the golangci-lint binary to the cache key would work here? And |
Most of the cost of a build can be skipped only with the Go cache (build and modules), so an extra cache (the binary) will not provide a major improvement. |
Another point, in this system there are 2 golangci-lint versions:
This can (and will) create confusion. |
Another element, the binary name is defined inside the This file can have several extensions: |
To be clear, I understand the need, it's interesting but it's not as simple as you describe in your proposal. |
Don't worry, it's your job, I certainly don't want to rush a solution! These are all very valid points. If we forget about the cache, do you think it would be acceptable to just:
This would result in relatively minor changes in the code of the action, and would "just work" for people using custom linters (I don't think anyone will randomly have a .custom-gcl.yml file in their repo) |
I think you didn't say that to minorize our work, but I recommend avoiding this type of estimation. You are a dev, like me, and we know that a change is often related to other elements. For me, your issue is related to #1049, this issue is about using a binary inside the PATH but this also requires thinking about local installation outside of the PATH (#631). Otherwise, it requires to handle the working directory. So even without caching the binary, the support of the custom build requires more important changes and more thinking than you can quickly evaluate. My previous comments were a way to share my thoughts during my thinking about the topic. |
Apologies for the miscommunication, I meant "relatively" as in "relatively compared to doing with the cache" (since dealing with the cache comes with more complexity as you explained above) ; not an absolute estimation of how much work this is |
To add a use case, I'm working on a linter for Kube API conventions (link), and I don't think that's a linter that would ever make sense to include within golangci-lint by default. However, golangci-lint is used in a lot of Kube related projects, and I built the linter in a way that it integrates, and works best, when used with I'm trying to demonstrate it within the Cluster API project at the moment. They use this action, and, given the performance benefits of the plugin, I didn't want to scrap using the plugin just to enable this extra custom linter. It has lead me to running the extra custom linter as a separate binary, separate GCL config, separate step in the workflow, without any of the benefits of the action. I think my work here would benefit from this RFE, and I'd be happy to help out, where appropriate in seeing this feature through. The only requirement I don't see already discussed is that we keep the |
Just as a workaround you can define a custom name: golangci-lint
description: Run golangci-lint in your Go project.
inputs:
version:
description: The golangci-lint version to use
required: false
custom-gcl-binary:
description: "The custom golangci-lint binary to use (default: ./build/custom-gcl)"
default: ./build/custom-gcl
required: false
skip-cache:
description: "Define if cache should be skipped (default: true)"
default: "true"
required: false
working-directory:
description: The working directory to run golangci-lint in
default: ""
required: false
runs:
using: "composite"
steps:
- name: Run golangci-lint-action
if: ${{ hashFiles(format('{0}/{1}',inputs.working-directory,'.custom-gcl.yml')) == '' }}
uses: golangci/golangci-lint-action@v6
with:
skip-cache: ${{ inputs.skip-cache }}
version: ${{ inputs.version }}
working-directory: ${{ inputs.working-directory }}
- name: Run custom-gcl linter
if: ${{ hashFiles(format('{0}/{1}',inputs.working-directory,'.custom-gcl.yml')) != '' }}
run: |
go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
golangci-lint custom
${{ inputs.custom-gcl-binary }} run ./...
shell: bash
working-directory: ${{ inputs.working-directory }}
May that is helpful to you as it was to us. |
Welcome
Your feature request related to a problem? Please describe.
golangci-lint features a way to support plugins automatically with a configuration file.
Basically it boils down to:
.custom-gcl.yml
config filegolangci-lint custom
.golangci.yml
./custom-gcl
instead of golangci-lintcustom-gcl
instead ofgolangci-lint
Describe the solution you'd like.
I'd like for the GitHub action to:
golangci-lint
as usual (no change to the normal flow).custom-gcl.yml
existsgolangci-lint custom
(or use the cached version)custom-gcl
using a hash sum of the.custom-gcl.yml
Describe alternatives you've considered.
I'm currently doing the above manually without using the GitHub action
Additional context.
I want to use nilaway as part of my CI, but it's only supported with golangci-lint via the plugin system. See: https://github.com/uber-go/nilaway?tab=readme-ov-file#golangci-lint--v1570
The text was updated successfully, but these errors were encountered: