Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for multiple plugins #717

Merged
merged 9 commits into from
Nov 7, 2022
Merged

add support for multiple plugins #717

merged 9 commits into from
Nov 7, 2022

Conversation

pkwarren
Copy link
Member

@pkwarren pkwarren commented Nov 4, 2022

Instead of requiring a 'lang' option to be specified, create separate
binaries configured to output code for a given language.

Instead of requiring a 'lang' option to be specified, create separate
binaries configured to output code for a given language.
@pkwarren pkwarren changed the title Pkw/multiple plugins add support for multiple plugins Nov 4, 2022
@@ -1,3 +1,4 @@
.git/
.idea/
.bin/
dist/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used by goreleaser (tested it locally to ensure it built all binaries).

@@ -1,3 +1,4 @@
# yaml-language-server: $schema=https://json.schemastore.org/github-workflow.json
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was being identified as both a github workflow and a goreleaser config file in vscode - this will remove invalid flagged errors when editing this file.

# python tooling for linting and uploading to PyPI
COPY requirements.txt .
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optimized the docker build to benefit from layer caching so we don't re-download the Python dependencies if any other files change.

@@ -1,107 +0,0 @@
# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This project is using Go modules - removed the Gopkg.* files used by dep.

@@ -25,7 +25,7 @@ build: validate/validate.pb.go ## generates the PGV binary and installs it into

.PHONY: bazel
bazel: ## generate the PGV plugin with Bazel
bazel build //tests/...
bazel build //cmd/... //tests/...
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verifies that the new commands can build w/ bazel.

optional := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL)
pgs.
Init(pgs.DebugEnv("DEBUG_PGV"), pgs.SupportedFeatures(&optional)).
RegisterModule(module.ValidatorForLanguage("cc")).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change (along with removing RegisterPostProcessor(pgsgo.GoFmt()). for non-Go generators).

@@ -73,6 +74,7 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>3.10.1</version>
<configuration>
<release>${java.release}</release>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that not only are source/target bytecode compatible with Java 8, but it also builds with the Java 8 bootclasspath to ensure no APIs introduced in Java 9+ are accidentally pulled in. It is supported by Java compilers 9+ (we are building with 17 as part of CI).

Ref: https://maven.apache.org/plugins/maven-compiler-plugin/compile-mojo.html#release

@pkwarren pkwarren requested a review from bufdev November 4, 2022 19:43
@pkwarren pkwarren requested a review from bufdev November 4, 2022 21:31
@pkwarren pkwarren marked this pull request as ready for review November 4, 2022 22:17
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