mszostok / codeowners-validator

The GitHub CODEOWNERS file validator
Apache License 2.0
217 stars 47 forks source link

rename test to comply with module.CheckFilePath #60

Closed sgmitchell closed 3 years ago

sgmitchell commented 3 years ago

Description Rename test and corresponding testdata file to appease go modules

Go modules will zip up everything in the repo, which results in files under testdata being run through CheckFilePath() which does not like ' in file names.

Removing that character from the testdata file (and corresponding tests) makes it happy. (change and working url)

An alternative solution proposed in multiple places is to just add an empty go.mod file to the testdata folder (change and working url)

Related issue(s) Fixes #59

codecov[bot] commented 3 years ago

Codecov Report

Merging #60 (4aeed2b) into master (085f270) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #60   +/-   ##
=======================================
  Coverage   50.13%   50.13%           
=======================================
  Files          12       12           
  Lines         375      375           
=======================================
  Hits          188      188           
  Misses        183      183           
  Partials        4        4           

Continue to review full report at Codecov.

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

mszostok commented 3 years ago

Hi @sgmitchell, thanks for fixing that problem! I wasn't aware of it.

Removing the ' makes sense as this is quite uncommon for filenames, so probably I shouldn't use it.

Regarding the option with the empty go.mod, I'm not sure AFIU this will result in downloading source code (tests also) without the testdata folder, which I know that in most use cases it's not used but still seems odd to me.

Provided solution LGTM 🚀