Skip to content

Conversation

@ductrung-nguyen
Copy link

@ductrung-nguyen ductrung-nguyen commented Dec 5, 2025

Description

Formatting the code properly

Key Changes

Testing and Verification

There is no change in the logic or in the tests

Related Issues

#1634

PR Checklist

  • [X ] Code changes adhere to the project's coding standards.
  • All tests pass locally.
  • [ X] The PR description follows the project's guidelines.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to fix code formatting issues using gofmt, primarily addressing import ordering and whitespace inconsistencies. However, it also includes undocumented dependency version updates that should ideally be separated into a different PR.

Key Changes:

  • Reordered imports in test files to follow Go's standard formatting (placing dot imports after named imports from the same package group)
  • Fixed trailing whitespace and comment alignment in source files
  • Updated several dependencies (ginkgo, gomega, logr, protobuf) and added new indirect dependencies

Reviewed changes

Copilot reviewed 10 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/smoke/smoke_test.go Reordered ginkgo imports per gofmt standards
test/smartstore/smartstore_test.go Reordered ginkgo imports per gofmt standards
test/smartstore/manager_smartstore_test.go Reordered ginkgo imports per gofmt standards
test/secret/secret_s1_test.go Reordered ginkgo imports per gofmt standards
test/secret/secret_m4_test.go Reordered ginkgo imports per gofmt standards
test/secret/secret_c3_test.go Reordered ginkgo imports per gofmt standards
test/secret/manager_secret_s1_test.go Reordered ginkgo imports per gofmt standards
test/secret/manager_secret_m4_test.go Reordered ginkgo imports per gofmt standards
pkg/splunk/enterprise/util.go Removed trailing whitespace on blank line
pkg/splunk/enterprise/afwscheduler_test.go Aligned inline comments for consistency
cmd/main.go Fixed whitespace alignment and removed trailing space from TODO comment
go.mod Updated dependency versions (should be separated from formatting changes)
go.sum Updated checksums reflecting dependency changes (should be separated from formatting changes)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@patrykw-splunk
Copy link
Collaborator

Hey, @ductrung-nguyen thanks for creating this PR. We will review it and get back to you as soon as possible

@ductrung-nguyen
Copy link
Author

thank you @patrykw-splunk.

Hope that this PR and #1639 can be reviewed and merged soon to avoid further conflict.
As I am gonna open several more (and bigger) PRs as discussed in the meeting between Amadeus & Splunk & Splunk operator team.

@kubabuczak
Copy link
Collaborator

kubabuczak commented Dec 11, 2025

Hi @ductrung-nguyen, thank for this PR.

One thing we need to make sure before merging is for you to sign CLA - can you fill out this form?
We will add automatic CLA check soon (Igor is working on it), but for now we need to do it manually.

@ductrung-nguyen
Copy link
Author

Thanks @kubabuczak
I tried to fill and submit the form, but when I click on "Submit", nothing happens.
The button is grey for a while, and then, nothing.

From developer console, I see that the response is "OK", but there are some errors on the UI:
image
image

@ductrung-nguyen
Copy link
Author

Not sure why the unit tests failed. There is no changes in the logic of the code.

@kubabuczak
Copy link
Collaborator

Regarding CLA sing up - I'm trying to contact people which may have more info about it - I'll get back to you as soon as I got something.

Regarding tests - there is a problem with secrets and tokens - workflows for PRs from external forks don't have access to them, so many tests won't pass. It's surprisingly complex to set up correctly.
I would say that maintainers should be able to vet code and say it's safe to execute, but GitHub doesn't provide easy way of configuring it in this way 🤷‍♂️.
Previously we copied the code to internal branch and execute tests in that way, but we should have some better solution going forward. I'll look into it.

@kubabuczak
Copy link
Collaborator

@ductrung-nguyen - sorry for long time waiting for this CLA issue. Just want to say that we are still working on it, but I have a hard time to get to the right people :/

@ductrung-nguyen
Copy link
Author

thank you very much. I completely understood the difficulties when setting the tests from external fork 😄

@kubabuczak
Copy link
Collaborator

kubabuczak commented Dec 18, 2025

@ductrung-nguyen can you try again to sign CLA? Web team said that the issue is resolved

@ductrung-nguyen
Copy link
Author

it works. I can sign. Thank you very much.

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