-
Notifications
You must be signed in to change notification settings - Fork 13
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
refactor: allow for clean-up and regenerating go files #1620
Conversation
Current dependencies on/for this PR: |
- name: Test generated files | ||
run: | | ||
make clean/go-generated && make generate | ||
git diff --exit-code |
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.
|
||
clean/go-generated: | ||
@echo "Cleaning generated .go files..." | ||
@find . -name '*.go' | xargs grep -l '// Code generated by .*; DO NOT EDIT.$$' | while read -r file; do echo ""$$file""; rm -f "$$file"; done |
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.
Related to although in that one we only check for mocks
fleetshard/pkg/runtime/runtime.go
Outdated
"github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager" | ||
fmImpl "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager/impl" |
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.
since there is just one occurence of fleetmanager
in this file I think we can swap this imports.
"github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager" | |
fmImpl "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager/impl" | |
fm "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager" | |
fleetmanager "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager/impl" |
@@ -7,6 +7,8 @@ import ( | |||
"github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/clusters/types" | |||
"github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/config" | |||
"github.com/stackrox/acs-fleet-manager/pkg/client/ocm" |
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.
If we replace interface idGenerator ocm.IDGenerator
we can remove this import and keep other namped as ocm
causing less changes
|
||
clustersmgmtv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" | ||
"github.com/pkg/errors" | ||
"github.com/stackrox/acs-fleet-manager/pkg/api" | ||
) | ||
|
||
const ( |
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.
this should be a separated PR with enabling unused
linter
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.
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.
If we work with naming then we can keep diff smaller on the other hand it's good to have it consistent. I'd like to remove changes not related to this PR.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 0x656b694d, janisz The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
80e6576
to
225f41a
Compare
New changes are detected. LGTM label has been removed. |
/retest |
clean/go-generated
Makefile target.Tested: