Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 24, 2025
Merged

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

merged 2 commits into from
May 24, 2025

Conversation

@StevenACoffman
Copy link
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
Contributor

Please fix:

  • the link in your PR description
  • unit tests

@generalmimon
Copy link
Contributor

@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
Contributor Author

StevenACoffman commented May 11, 2025 •
edited
Loading

Thanks! I think I have it updated correctly now.

@codecov
Copy link

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
Contributor

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

@StevenACoffman
Copy link
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
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
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