Skip to content

Update default excludes config.go to include Golang go.work and go.work.sum#468

Merged
theoludwig merged 2 commits intoeditorconfig-checker:mainfrom
StevenACoffman:main
May 24, 2025
Merged

Update default excludes config.go to include Golang go.work and go.work.sum#468
theoludwig merged 2 commits intoeditorconfig-checker:mainfrom
StevenACoffman:main

Conversation

@StevenACoffman
Copy link
Copy Markdown
Contributor

@StevenACoffman StevenACoffman commented May 9, 2025 •
edited
Loading

Per this discussion editorconfig/editorconfig-core-go#210 (comment) and suggestion from @generalmimon

This updates the default https://github.com/editorconfig-checker/editorconfig-checker/tree/f1ae05b9aee4fd6404a398687cb05cf1d8df4158?tab=readme-ov-file#default-excludes

Linting with editorconfig-checker (or super-linter which uses that) complained about our go.work and go.work.sum) lead me here.

If you're looking for a solution for editorconfig-checker specifically, I think the right way would be to make a pull request that adds go.work{,.sum} to defaultExcludes. The go.{mod,sum} files are already there, so it makes a lot of sense:

config.go:19-23

// DefaultExcludes is the regular expression for ignored files
var DefaultExcludes = strings.Join(defaultExcludes, "|")

// defaultExcludes are an array to produce the correct string from
var defaultExcludes = []string{

config.go:34-35

	// Go Modules (Go)
	"go\\.(mod|sum)$",

These "default excludes" are also mentioned in the README: editorconfig-checker / Default Excludes

@ccoVeille
Copy link
Copy Markdown
Contributor

ccoVeille commented May 9, 2025

Please fix:

  • the link in your PR description
  • unit tests

@generalmimon
Copy link
Copy Markdown
Contributor

generalmimon commented May 9, 2025

@ccoVeille:

  • unit tests

Yes, and I suppose the README.md should be updated too. #324 looks like a good PR for inspiration on how it should look.

@StevenACoffman StevenACoffman force-pushed the main branch 2 times, most recently from 65545a4 to b8faadf Compare May 11, 2025 14:30
@StevenACoffman
Copy link
Copy Markdown
Contributor Author

StevenACoffman commented May 11, 2025 •
edited
Loading

Thanks! I think I have it updated correctly now.

@codecov
Copy link
Copy Markdown

codecov bot commented May 11, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.59%. Comparing base (af09b21) to head (845f5cb).
Report is 54 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #468      +/-   ##
==========================================
+ Coverage   86.72%   87.59%   +0.86%     
==========================================
  Files          11       11              
  Lines        1017     1048      +31     
==========================================
+ Hits          882      918      +36     
+ Misses        102       99       -3     
+ Partials       33       31       -2

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ccoVeille
Copy link
Copy Markdown
Contributor

ccoVeille commented May 11, 2025

Tests are OK, but you need to update the commits message to comply with lint-commit action

@StevenACoffman
feat: add go.work, and go.work.sum files to default excludes
5a11aa4 
Signed-off-by: Steve Coffman <steve@khanacademy.org>
@StevenACoffman
Copy link
Copy Markdown
Contributor Author

StevenACoffman commented May 11, 2025 •
edited
Loading

Ok, I git reset --soft to the last common commit, and then re-commit using the commit from the example PR but updated to have the relevant file names. After force pushing I hope it will now match the lint-commit action

ccoVeille
ccoVeille approved these changes May 12, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks for this

@StevenACoffman
Copy link
Copy Markdown
Contributor Author

StevenACoffman commented May 13, 2025 •
edited by klaernie
Loading

Thanks for this

Ha, this was nothing. Thank you all for this project (and a bunch more)!

@theoludwig theoludwig merged commit 05296a5 into editorconfig-checker:main May 24, 2025
6 checks passed
@editorconfig-checker-bot editorconfig-checker-bot mentioned this pull request May 24, 2025
Merged
@BrewTestBot BrewTestBot mentioned this pull request Aug 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ccoVeille ccoVeille ccoVeille approved these changes

@klaernie klaernie klaernie approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@StevenACoffman @ccoVeille @generalmimon @klaernie @theoludwig