Skip to content

feat: add go fmt to make fmt command#1178

Merged
nashqueue merged 2 commits intomainfrom
sevey/add-go-fmt
Sep 13, 2023
Merged

feat: add go fmt to make fmt command#1178
nashqueue merged 2 commits intomainfrom
sevey/add-go-fmt

Conversation

@MSevey
Copy link
Copy Markdown
Contributor

@MSevey MSevey commented Sep 12, 2023

Overview

Closes #223

Instead of adding a formatted to the CI, which would introduce a diff that would need to be committed, I extended the make fmt command.

@MSevey MSevey added the T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. label Sep 12, 2023
@MSevey MSevey marked this pull request as ready for review September 12, 2023 12:58
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 12, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.38% ⚠️

Comparison is base (472f245) 56.08% compared to head (30e2a49) 55.71%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1178      +/-   ##
==========================================
- Coverage   56.08%   55.71%   -0.38%     
==========================================
  Files          63       63              
  Lines        6711     6711              
==========================================
- Hits         3764     3739      -25     
- Misses       2560     2583      +23     
- Partials      387      389       +2     

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Manav-Aggarwal
Manav-Aggarwal previously approved these changes Sep 12, 2023
@gupadhyaya
Copy link
Copy Markdown
Contributor

gupadhyaya commented Sep 12, 2023

@MSevey can we inherit from app or node?
e.g., node uses gofmt, goimports along with markdownlint https://github.com/celestiaorg/celestia-node/blob/main/Makefile#L83
app just uses golangci-lint with --fix: https://github.com/celestiaorg/celestia-app/blob/main/Makefile#L93

either we can just do @golangci-lint run --fix or combination of gofmt and goimports like node does. i prefer earlier as I used it in the past.

@MSevey
Copy link
Copy Markdown
Contributor Author

MSevey commented Sep 13, 2023

@MSevey can we inherit from app or node? e.g., node uses gofmt, goimports along with markdownlint https://github.com/celestiaorg/celestia-node/blob/main/Makefile#L83 app just uses golangci-lint with --fix: https://github.com/celestiaorg/celestia-app/blob/main/Makefile#L93

either we can just do @golangci-lint run --fix or combination of gofmt and goimports like node does. i prefer earlier as I used it in the past.

I don't think we can inherit another repo's makefile in a clean way. But happy to use the golangci-lint --fix flag 👍

@nashqueue nashqueue added this pull request to the merge queue Sep 13, 2023
Merged via the queue into main with commit 96848dc Sep 13, 2023
@nashqueue nashqueue deleted the sevey/add-go-fmt branch September 13, 2023 14:56
gupadhyaya pushed a commit that referenced this pull request Sep 13, 2023
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->

Closes #223 

Instead of adding a formatted to the CI, which would introduce a diff
that would need to be committed, I extended the `make fmt` command.
chandiniv1 pushed a commit to chandiniv1/rollkit that referenced this pull request Sep 25, 2023
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->

Closes evstack#223 

Instead of adding a formatted to the CI, which would introduce a diff
that would need to be committed, I extended the `make fmt` command.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add go format to CI

4 participants