vektah / dataloaden

go generate based DataLoader

MIT License

528 stars 79 forks source link

Fixes issue where dataloader package would be a test package #15

Closed codyleyhan closed 5 years ago

codyleyhan commented 5 years ago

Noticed this issue popping up when I was generating my loaders.

I think it highly depends on the order of the range for the map after parser.ParseDir is called

codecov[bot] commented 5 years ago

Codecov Report

Merging #15 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #15   +/-   ##
=======================================
  Coverage   86.51%   86.51%           
=======================================
  Files           2        2           
  Lines          89       89           
=======================================
  Hits           77       77           
  Misses         11       11           
  Partials        1        1

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 314ac81...fb9c020. Read the comment docs.

vektah commented 5 years ago

Thanks for the pr, I hadn't though of this. I wonder if the same thing happens for random main packages that are excludes using build tags. _test files are a magic build tag at some level too right?

codyleyhan commented 5 years ago

Do you think the better approach would be to just allow the user to override the package with a flag?

vektah commented 5 years ago

Does the parser return build tags, or have an option to ignore non matching build tags?

codyleyhan commented 5 years ago

Does not appear so, https://golang.org/pkg/go/ast/#Package is returned which just has basic information, while https://golang.org/pkg/go/build/#Package has the information that may be needed. Maybe move toward using this package instead?

vektah commented 5 years ago

Thanks, that looks pretty clean :+1: