Drop cobra dependency #42
Conversation
End-rey
requested review from
carpawell,
cthulhu-rider,
roman-khimov and
AliceInHunterland
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #42 +/- ##
==========================================
+ Coverage 25.94% 28.75% +2.80%
==========================================
Files 23 21 -2
Lines 767 793 +26
==========================================
+ Hits 199 228 +29
- Misses 533 540 +7
+ Partials 35 25 -10 ☔ View full report in Codecov by Sentry. |
Use default flag package instead of cobra. Closes #34. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
Now the binary code is not created, but just runs through go run. Also removed unnecessary things and improved the code style. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
End-rey
force-pushed
the
34-drop-cobra-dependency
branch
from
6f6a9bf to
61a066c
Compare
internal/generate/generate.go
Outdated
| flag.StringVar(&locodeGenerateOutPath, locodeGenerateOutputFlag, "", "Target path for generated database (directory))") | ||
| } | ||
|
|
||
| func Execute() { |
There was a problem hiding this comment.
@End-rey now it's a main pkg in internal/ containing executable program which is usually placed in cmd/, may be move it there in concert?
There was a problem hiding this comment.
may be move it there in concert?
but why? i think it is internaled intentionally. no one is expected to use it, it is for our generations only
There was a problem hiding this comment.
cuz its a common approach across various Go projects. I can use it from intenal as well, and placing program into cmd does not force usage
There was a problem hiding this comment.
@cthulhu-rider Do you propose to make such structure?
.
├── cmd
│ └── generate
│ └── main.go
├── internal
└── pkg
I agree that then the structure will be better and the command code will be separated from the library code.
But why haven't we used such a structure before? Were there any significant reasons for this?
There was a problem hiding this comment.
such structure?
exactly
But why haven't we used such a structure before?
idk, this isnt a big problem so no one cared i guess
There was a problem hiding this comment.
That's mostly because this command is completely internal, it's supposed to be used only by package developers. And the only thing properly exposed to the outside world is pkg/locodedb.
There was a problem hiding this comment.
That's mostly because this command is completely internal, it's supposed to be used only by package developers. And the only thing properly exposed to the outside world is
pkg/locodedb.
That is, to leave the structure as it is now? main.go in internal?
There was a problem hiding this comment.
it's weird to have main package in internal, at least it should be internal/generate. With this, it's easy to make it cmd/generate and not to go against the generally accepted approach
Move all from generate.go to main.go and make Execute main function. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
Use flag.Func to obtain string slice instead of flag.Var. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
Resolve problem with specific os PATH. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
Signed-off-by: Andrey Butusov <andrey@nspcc.io>
Close #34.
Use default flag package instead of cobra and use
go runinstead of build binary in Makefile.