Closed mattlqx closed 3 years ago
I'll take a pass at codeclimate feedback tomorrow.
Slightly refactored for codeclimate feedback. Not sure what to do with the test failing version check for dirty vs. not. I think this is in a good human review spot though. 👍🏻
@mattlqx this looks great! Thank you :bow:
Concerning the failing test: For some reason, git describe --tags --always --dirty
returns a different value in the test suite than in the Makefile. It should work when slightly adjusting Makefile
and print-version.bats
It seems I cannot highlight/suggest changes here in files which were not changed. Alternatively, I submitted a PR to your branch to highlight the diff - no need to merge that :)
@mattlqx it seems go.mod
is not up-to-date. When I run make compile
locally from the latest commit, then I see that go.mod
is changed --> that change results in -dirty
suffix.
That also explains why git describe --tags --always --dirty
differs at compile and at test time.
Compile is run first, when the branch is clean, i.e., no -dirty
flag. The compile changes go.mod
file. Now integration test runs, and git describe --tags --always --dirty
sees the change in go.mod
file and adds -dirty
suffix.
Thanks, I'm still a noob on golang dependency stuff. A go get -u
should've been enough to fix that, right?
Afaik go get -u
updates all dependencies - also the already locked ones.
make compile
(go build
) will update the go.mod
file automatically if sth is missing - but it will not update existing deps.
I think the easiest way to resolve this, is to get the dep state from latest master
again and run make compile
.
One way this could be done:
git checkout 59_replace
rm -r vendor/ go.mod go.sum
git checkout master
cp -r vendor/ /tmp
, cp go.mod /tmp
, cp go.sum /tmp
git checkout 59_replace
mv /tmp/vendor/ .
mv /tmp/go.mod .
mv /tmp/go.sum .
make compile
git add vendor/
I will have a look at updating all dependencies on a different branch later. You dont need to fight through that :)
Thanks. Looks like it's finally gotten past it. Maybe had something to do with my local go 1.16beta version on this Mac M1. I used a docker container of go 1.15.7 to build. Also just fyi, git checkout master go.sum go.mod vendor/
does the same thing as all those steps you listed. :)
Wow git checkout
is more powerful than I thought. Learned sth new today. Thx for the tip! It will save me a lot of time in the future :)
To be very honest the way I handle the deps here is also outdated afaik. You shouldn't take that as a positive example of how to do stuff in go :)
Nowadays most people dont include the vendor/
dir anymore, as it bloats up the repository. However, I actually like the static aspect of it. Further, it is already in the git history and for this project it doesn't bother me too much, so I keep it.
I will merge this PR later tonight and make a new release. Again, thank you @mattlqx for these awesome contributions. You have improved vsh
a lot :bow:
Thanks! This is going to be a very useful tool for managing our application configurations so I'm happy to help.
The
replace
command will do a match in the same fashion asgrep
but instead of presenting results with search patterns highlighted, it will highlight matches in red followed immediately by replacement string in green. In this way, it is easy to identify on one line what will be matched and replaced. by default, the operation will ask for confirmation before writing back to vault. it is possible to use flags-y
to write without confirmation,-n
to skip confirmation and not write aka dry-run, and limit scope to keys (-k
) and values (-v
) in the same way as ingrep
.Moves search functions to its own file and provides a struct common for search/replace operations. The struct is embedded with commands that utilize search so that the commands contain the search paramters and the commands implement an interface for retrieving the parameters. This allows common methods to be located on a
Searcher
struct that can reference the command via the interface.There's a lot of fat that can be trimmed from this implementation. I'm not super happy with it, but I think this is the kind of functionality I originally envisioned with #59 . I think in the future, it'd be nice to give the option of a traditional diff display instead of strictly inline color highlighting.