Skip to content

gofmt all files #53

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

gofmt all files #53

wants to merge 2 commits into from

Conversation

andig
Copy link

@andig andig commented Jul 6, 2025

Follow-up to #19

While gofmt is opinionated, it seems good practice to use it.

@Copilot Copilot AI review requested due to automatic review settings July 6, 2025 13:13
Copy link

@Copilot 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 reformats code across the repository using go fmt, updating variable declarations to use short forms and simplifying composite literals where possible.

  • Converted var declarations to short declarations (:=)
  • Updated composite literals in tests to use implicit type inference
  • Adjusted brace and comma placement to match Go formatting conventions

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
decode_test.go Reformatted test slice and map literals; no logic changes
decode.go Switched from var x = to x := for scalar parsing vars

@andig andig changed the title go fmt all files gofmt all files Jul 6, 2025
@WGH-
Copy link

WGH- commented Jul 7, 2025

Is this really gofmt/go fmt, though? I believe the source has already been reformatted a couple of commits ago (5f3d8c3).

@andig
Copy link
Author

andig commented Jul 7, 2025

It's https://github.com/mvdan/gofumpt with default settings which (my) Vscode is happy to apply by default. I consent it's optioned, please close if not appropriate.

@ingydotnet
Copy link
Member

This is fine but we'll wait to get a few PRs merged.

In the meantime, can you change your commit message to:

  • Start with a capital letter
  • Not end with a period
  • Be 50 chars or less

(I know we need to add these rules to Contributing and enforce them with PR checks...)

# Conflicts:
#	decode_test.go
@andig
Copy link
Author

andig commented Jul 8, 2025

@ingydotnet with all due respect:

Start with a capital letter

gofmpt is a command and should not be capitalized. If you need something like Use gofmt... could you kindly rephrase when squashing the commits? I feel this round trip is a bit more bureaucratic than could be done?

Thank you :)

That said and less it is skipped: this PR removes https://github.com/yaml/go-yaml/pull/53/files#diff-373fb23355f8ed601aa2d3c450033ccd8cf067b36ae6255cb9d2af857bf71f55L1115-L1118 since it's unused- might be an oversight from before?

@ingydotnet
Copy link
Member

ingydotnet commented Jul 9, 2025

@ingydotnet with all due respect:

Start with a capital letter

gofmpt is a command and should not be

I certainly didn't mean for you to capitalize that just to rephrase things so that a word came before that.

These are just the preliminary rules I'm putting in place that match the style largely used so far. Like I said before we can revisit all of the specifics after we are in a reliable state.

But I will do this for you certainly, when I merge the PR. 👍

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.

3 participants