Skip to content
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

Go Code format checker and formatter #171

Closed
wants to merge 1 commit into from

Conversation

faraazbrcm
Copy link

Also formatted the files which dont adhere to go formats. Build will fail if there exists any formatting issue

@faraazbrcm faraazbrcm force-pushed the master branch 4 times, most recently from 5f11925 to f59552f Compare November 6, 2023 05:23
@xumia
Copy link
Contributor

xumia commented Nov 14, 2023

It is good to bypass the coverage check for only the format change.

@faraazbrcm
Copy link
Author

faraazbrcm commented Nov 14, 2023 via email

Makefile Outdated
FORMAT_CHECK = $(BUILD_DIR)/.formatcheck
FORMAT_LOG = $(BUILD_DIR)/go_format.log
# Find all .go files excluding vendor, build, and patches files
GO_FILES := $(shell find . -type f -name '*.go' ! -path './vendor/*' ! -path './build/*' ! -path './patches/*' ! -path './proto/*')
Copy link

@kwangsuk kwangsuk Nov 14, 2023

Choose a reason for hiding this comment

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

In case of swsscommon, the go file is auto-generated by a foolproof swig tool, which does not require rigorous go formatting. So, can you add 'swsscommon/*' to the excluded paths?

Makefile Show resolved Hide resolved
Copy link
Contributor

@rlucus rlucus left a comment

Choose a reason for hiding this comment

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

Approval from Google.

Also formatted the files which dont adhere to go formats. Build will fail if there exists any formatting issue
@faraazbrcm
Copy link
Author

Replaced with #182 as it supports formatting using Bookworm's Golang version(go1.19)

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