Skip to content

api: Remove confusing line about auto-generation#17536

Merged
htuch merged 1 commit intoenvoyproxy:mainfrom
phlax:api-build-confusion
Jul 29, 2021
Merged

api: Remove confusing line about auto-generation#17536
htuch merged 1 commit intoenvoyproxy:mainfrom
phlax:api-build-confusion

Conversation

@phlax
Copy link
Member

@phlax phlax commented Jul 29, 2021

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: api: Remove confusing line about auto-generation
Additional Description:

afaiaa this file is not auto-generated (grepping code appears to confirm that)

i think these messages are present in the api directory so that they are copied over to generated_api_shadow - or perhaps were copied from there at some stage

either way - im pretty sure the message removed here is wrong

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Jul 29, 2021

cc @htuch @mattklein123

@htuch htuch merged commit 042b882 into envoyproxy:main Jul 29, 2021
@mattklein123
Copy link
Member

This file is auto generated:

# Generate api/BUILD file based on updated type database.
bazel build "${BAZEL_BUILD_OPTIONS[@]}" //tools/type_whisperer:api_build_file
cp -f bazel-bin/tools/type_whisperer/BUILD.api_build_file api/BUILD

@phlax
Copy link
Member Author

phlax commented Jul 29, 2021

This file is auto generated:

is that not for comparison - to check there is no diff ?

@phlax
Copy link
Member Author

phlax commented Jul 29, 2021

one thing im a bit concerned about is that when any api changes its going to expect that header at the top that was removed

i kinda meant this pr more as an rfc to try and clarify the situation

ill experiment further with api changes

@mattklein123
Copy link
Member

is that not for comparison - to check there is no diff ?

No, I don't think so. I'm still trying to sort through all of this. It's incredibly confusing and contrib makes it more so.

baojr added a commit to baojr/envoy that referenced this pull request Jul 29, 2021
…bridge-stream

* upstream/main: (140 commits)
  quiche: remove google quic support (envoyproxy#17465)
  runtime: removing envoy.reloadable_features.check_ocsp_policy (envoyproxy#17524)
  upstream: not trying to do HTTP/3 where not configured (envoyproxy#17454)
  api: Remove confusing line about auto-generation (envoyproxy#17536)
  v2: removing bootstrap (envoyproxy#17523)
  connpool: Fix crash in pool removal if the cluster was already deleted (envoyproxy#17522)
  Enhance the comments clearly (envoyproxy#17517)
  mysql proxy: connection attributes parsing  (envoyproxy#17209)
  [ci] fix false positive CVE scan from node (envoyproxy#17510)
  Fixing Envoy Mobile factory strings (envoyproxy#17509)
  http3: validating codec (envoyproxy#17452)
  quic: add QUIC upstream stream reset error stats (envoyproxy#17496)
  thrift proxy: move UpstreamRequest into its own file (envoyproxy#17498)
  docs: Fixed FaultDelay docs. (envoyproxy#17495)
  updates links to jaegertracing-plugin.tar.gz (envoyproxy#17497)
  http: make custom inline headers bootstrap configurable (envoyproxy#17330)
  deps: update yaml-cpp to latest master (envoyproxy#17489)
  improving tracer coverage (envoyproxy#17493)
  Increase buffer size of `Win32RedirectRecords` (envoyproxy#17471)
  ext_proc: Fix problem with buffered body mode with empty or no body (envoyproxy#17430)
  ...

Signed-off-by: Garrett Bourg <bourg@squareup.com>
@mattklein123
Copy link
Member

FYI, this line gets re-added for me locally, so, it's going to get re-added in the next commit that changes API I would imagine.

phlax added a commit to phlax/envoy that referenced this pull request Jul 30, 2021
… there

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Jul 30, 2021

ive opened a pr to revert #17548

mostly i was trying to demonstrate that while proto_format enforces a format and does a comparison with generated_api_shadow its not used in any way building the docs (or for anything else i think) - its a formatting/integrity test

the docs run in presubmit before proto_format is run, and in postsubmit independently

@phlax
Copy link
Member Author

phlax commented Jul 30, 2021

i guess i can see how it might be regarded as auto-generated

my own thinking is that its auto-generated in the way that eg gofmt auto-generates files, altho i guess in this case its formatting from the actual deps, etc

i think it comes down to a matter of perspective, personally i edit this file when making related changes, but if you followed a workflow of make changes, run proto_format, then it would be auto-generated

really not trying to cause any confusion, just again trying to emphasize that from envoy source proto_format is not part of the pipeline to build anything

phlax added a commit that referenced this pull request Jul 30, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Signed-off-by: Ryan Northey <ryan@synca.io>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
… there (envoyproxy#17548)

Signed-off-by: Ryan Northey <ryan@synca.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants