Improve initial developer experience#473
Conversation
| export PATH := $(BIN):$(PATH) | ||
| export GOBIN := $(abspath $(BIN)) | ||
| export PYTHONPATH ?= gen | ||
| BUF_VERSION := 1.69.0 |
There was a problem hiding this comment.
While there is a slight issue in having this version and the buf version in pyproject.toml, the version of the license header tool isn't that important, and the correct fix will be in the future, using the Python version of the tool we will share among Buf's repos
| .PHONY: upstream | ||
| upstream: $(BIN)/buf | ||
| rm -rf upstream | ||
| $(BIN)/buf export $(PROTOVALIDATE_PROTO_PATH) -o upstream/proto |
There was a problem hiding this comment.
@timostamm I have removed this, looking at #409 I couldn't find a particular reason for vendoring in the proto file itself with this line. We have $(BIN)/buf generate $(PROTOVALIDATE_PROTO_PATH) in the generate script, and then run $(BIN)/buf generate below it. The later does use this proto file to effectively invalidate the former, but it seems enough to use the form that automatically gets the proto from BSR or GitHub without this? Let me know if you remember there was any other reason for keeping this here
There was a problem hiding this comment.
What we need for development:
- Generate
validate.protofor the validator implementation -proto/protovalidate/in upstream - Generate conformance test protos -
proto/protovalidate-testing/in upstream - Run the Go conformance test harness -
tools/protovalidate-conformance/in upstream - Local test protos to cover implementation-specific edge cases -
proto/testsin this repository - importing upstream'svalidate.proto
Because upstream's Protobuf definitions are not pushed to the BSR until release, we need the ability to reference arbitrary upstream Git commits. This enables testing changes to validate.proto in implementations before cutting an upstream release - which would be immediately available to users, but not supported by implementations yet. In practice, we need to have all implementations ready to release when we are cutting an upstream release.
If you pull an upstream change in here, it is very possible that you encounter a conformance test failure and need to investigate the problem further. You'll need to add local test protos, and sometimes you may want to modify validate.proto with a patch that you expect will fix the problem, but want to verify locally before going through an upstream commit.
The setup in protovalidate-es checks all boxes. I'm not sure that the setup here ever did. Are there any gaps in the new setup?
There was a problem hiding this comment.
Thanks for the context!
you may want to modify validate.proto with a patch that you expect will fix the problem, but want to verify locally before going through an upstream commit.
This makes sense. This means probably the step that generated code with upstream before downloading locally and generating again had both a redundant generation step and possibly a frustrating overwrite of local changes when not desired. Will tweak to resolve these.
There was a problem hiding this comment.
It's a "may", and some complications with that are acceptable. But keep in mind that local test protos cannot import from a Git commit.
There was a problem hiding this comment.
Thanks. I have filed
to track this. I would like to fix forward (technically refix) since many things will open up when migrating off of Makefile and in the meantime, the repo not being easily openable in an IDE should be fixed first. I'll address it soon.
This is extracted from #426. There was initially a plan to move repos completely but that didn't happen so we can focus on some devx improvements here. There will be much more coming like migrating to poe, but this addresses the major one, where when opening the repo, IDEs can't resolve the code in the
genfolder, causing red lines within the logic inprotovalidate/**and needing to hack PYTHONPATH to get things wired up. Initially I had proposed using generated SDKs to remove gencode from here completely, but #426 (comment) makes it seem we should still have the ability to run with a prerelease proto when needed. So this justtest/gensince they are test-onlygo installtogo runwhich is as fast for recent Go versions without having to manage paths