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

Skip .git folder when searching for *.go files #6118

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

Groxx
Copy link
Member

@Groxx Groxx commented Jun 5, 2024

Apparently that was being searched, and it noticed a "thing.go" folder that git made!
So now this has two improvements:

  1. it does not step into the .git folder, because obviously there is no source that needs formatting in there
  2. it only finds files, -type f, because directories aren't source files

Both pretty obviously good to have in retrospect.


A way to validate this kind of change for the future:

  1. change the SHELL = ... line near the top of the makefile to include a -x debug flag.
    make will now print all the $(shell ...) commands it runs, including this find.
  2. copy it and run it by hand and check the output.
    in particular, this time I just had it find all files to check where it looked, and made sure the list was reasonable
    (it included my local .idea, but that seems fine, it's small and has no go files)

I should've done 2 earlier when I added this find command, I likely would have noticed the directories and .git and removed them. Sorry about that.

Apparently that was being searched, and it cnoticed a "thing.go" folder that git made!
So now this has two improvements:
1. it does not step into the .git folder, because obviously there is no source that needs formatting in there
2. it only finds files, `-type f`, because directories aren't source files

Both pretty obvious in retrospect.
@Groxx Groxx enabled auto-merge (squash) June 5, 2024 23:11
@Groxx Groxx disabled auto-merge June 5, 2024 23:12
@Groxx Groxx enabled auto-merge (squash) June 5, 2024 23:12
@Groxx Groxx merged commit 741ff00 into cadence-workflow:master Jun 5, 2024
18 checks passed
Copy link

codecov bot commented Jun 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.96%. Comparing base (590500c) to head (409fef7).
Report is 6 commits behind head on master.

Additional details and impacted files

see 10 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 590500c...409fef7. Read the comment docs.

@coveralls
Copy link

coveralls commented Jun 5, 2024

Pull Request Test Coverage Report for Build 018feaa8-4d2b-4152-bf5a-38f1696b2d57

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 521 unchanged lines in 13 files lost coverage.
  • Overall coverage decreased (-0.3%) to 70.858%

Files with Coverage Reduction New Missed Lines %
service/history/shard/context.go 2 78.36%
service/matching/tasklist/task_list_manager.go 2 76.45%
service/history/task/transfer_active_task_executor.go 2 72.77%
common/membership/hashring.go 2 84.69%
service/matching/tasklist/matcher.go 2 89.35%
common/task/fifo_task_scheduler.go 3 84.54%
service/history/task/fetcher.go 3 86.6%
common/types/mapper/thrift/shared.go 4 98.31%
service/history/task/transfer_standby_task_executor.go 6 87.35%
service/history/replication/task_processor.go 7 81.25%
Totals Coverage Status
Change from base Build 018feaa3-8a65-4c9c-874c-0243b9141bed: -0.3%
Covered Lines: 105538
Relevant Lines: 148942

💛 - Coveralls

timl3136 pushed a commit to timl3136/cadence that referenced this pull request Jun 6, 2024
Apparently that was being searched, and it noticed a "thing.go" folder that git made!
So now this has two improvements:
1. it does not step into the `.git` folder, because obviously there is no source that needs formatting in there
2. it only finds files, `-type f`, because directories aren't source files

Both pretty obviously good to have in retrospect.

---

A way to validate this kind of change for the future:
1. change the `SHELL = ...` line near the top of the makefile to include a `-x` debug flag.
   `make` will now print all the shell commands it runs, including this `find`.
2. copy it and run it by hand and check the output.
   in particular, this time I just had it find _all_ files to check where it looked, and made sure the list was reasonable
   (it included my local .idea, but that seems fine, it's small and has no go files)

I should've done 2 earlier when I added this `find` command, I likely would have noticed the directories and `.git` and removed them.  Sorry about that.
timl3136 pushed a commit to timl3136/cadence that referenced this pull request Jun 6, 2024
Apparently that was being searched, and it noticed a "thing.go" folder that git made!
So now this has two improvements:
1. it does not step into the `.git` folder, because obviously there is no source that needs formatting in there
2. it only finds files, `-type f`, because directories aren't source files

Both pretty obviously good to have in retrospect.

---

A way to validate this kind of change for the future:
1. change the `SHELL = ...` line near the top of the makefile to include a `-x` debug flag.
   `make` will now print all the shell commands it runs, including this `find`.
2. copy it and run it by hand and check the output.
   in particular, this time I just had it find _all_ files to check where it looked, and made sure the list was reasonable
   (it included my local .idea, but that seems fine, it's small and has no go files)

I should've done 2 earlier when I added this `find` command, I likely would have noticed the directories and `.git` and removed them.  Sorry about that.
@Groxx Groxx deleted the skip-dot-git branch June 6, 2024 19:20
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