search

omec-project / smf

9 stars 33 forks source link

Fix non-functional linting warnings #183

Closed gatici closed 9 months ago

gatici commented 10 months ago

Fixing whitespace and import order warnings by running following command. This PR does not affect the code functionality.

golangci-lint run --fix
onf-bot commented 10 months ago

Can one of the admins verify this patch?

onf-bot commented 10 months ago

Can one of the admins verify this patch?

gab-arrobo commented 10 months ago

retest this please

thakurajayL commented 10 months ago

Do you need this to be reviewed ?

gab-arrobo commented 9 months ago

test this please

gab-arrobo commented 9 months ago

retest this please

gab-arrobo commented 9 months ago

@gatici, I know why the pull_request fails and you have a PR that addresses the unit-test-pfcpiface (#180), but I was expecting this PR to pass the golangci-lint test/GHA. Am I overlooking something?

gab-arrobo commented 9 months ago

@gatici, I know why the pull_request fails and you have a PR that addresses the unit-test-pfcpiface (#180), but I was expecting this PR to pass the golangci-lint test/GHA. Am I overlooking something?

@gatici, please double check this PR because it looks like golangci-lint run --fix did not properly fix linting warnings and actually added more issues. For example, in context/sm_contest.go, imports are now duplicated as shown below:

    "net"
    "net/http"
    "net"
    "net/http"

I am making a couple modifications to see if the issue can be directly addressed from the PR

gab-arrobo commented 9 months ago

test this please

gab-arrobo commented 9 months ago

Fixing whitespace and import order warnings by running following command. This PR does not affect the code functionality.

golangci-lint run --fix

@gatici, how did you install golangci-lint? I installed it through snap and from go but I cannot replicate your PR by running golangci-lint run --fix through any of these two options.

gatici commented 9 months ago

golangci-lint

Hi @gab-arrobo I installed it through the link by getting the binary, then run the command golangci-lint run --fix.

gab-arrobo commented 9 months ago

golangci-lint

Hi @gab-arrobo I installed it through the link by getting the binary, then run the command golangci-lint run --fix.

Hi @gatici, thanks for the link. Based on what I am reading, golangci-lint is the same as this golangci-lint-action GHA, which is the recommended option for CI/CD projects as indicated here. So, I am running the GitHub Action but it does not make any fixes/changes to the files. Would it be that your PR includes lots of changes because the way you are running the command (golangci-lint run --fix) and this does not take into account the options provided in .golangci.yml.

Moreover, when downloading the binary from the link you provided, I still cannot replicate your PR. Here are the steps I followed:

$ curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.2
$ golangci-lint --version
golangci-lint has version 1.55.2 built with go1.21.3 from e3c2265f on 2023-11-03T12:59:25Z
$ cd smf/
$ golangci-lint run --fix
$ git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
gatici commented 9 months ago

Hello @gab-arrobo

After rebasing on master, I can not reproduce it. The reason is https://github.com/omec-project/smf/pull/182/files# is disabled all the linters in .golangci.yml. After merging PR 182, this PR become invalid. I will close it at the moment.