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

Don't include protos (and some other dependencies) in jars #251

Closed
wants to merge 2 commits into from

Conversation

smparkes
Copy link
Contributor

The cel target in publish does not exclude the googleapis protobufs and so the generated classes get included in the jar. Some other stuff like com.google.rpc.Status also get pulled in. This causes problems when trying to use the maven central artifact in builds that include the googleapis artifacts from maven central.

With this change, only dev.cel.* and cel.* classes are included in the artifact.

@l46kok l46kok added the ready to pull Pulls the pending PR into Critique label Feb 23, 2024
Copy link
Collaborator

@l46kok l46kok left a comment

Choose a reason for hiding this comment

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

Looks good, just a few nits:

publish/BUILD.bazel Show resolved Hide resolved
@smparkes
Copy link
Contributor Author

Thanks for the quick review.

copybara-service bot pushed a commit that referenced this pull request Feb 27, 2024
@l46kok
Copy link
Collaborator

l46kok commented Feb 27, 2024

Merged a370d05. Thanks for the PR

@l46kok l46kok closed this Feb 27, 2024
@smparkes
Copy link
Contributor Author

Thanks, @l46kok.
Any thoughts on a deployment to maven central?
I can create a branch off of the 0.3.0 commit and cherry pick the change if we want a 0.3.1 instead of a 0.4.0?
(We do have a workaround if there's a need to delay releasing ...)

@smparkes smparkes deleted the smparkes/publish branch February 27, 2024 20:18
@l46kok
Copy link
Collaborator

l46kok commented Feb 27, 2024

Cadence wise, we're targeting for a quarterly release at a minimum. I'm planning on cutting a 0.4.0 release once all of the Subexpression optimization related changes are in (likely around end of March).

If you need a release sooner than that, feel free to cherry pick and create a 0.3.1 branch. I'd be happy to publish to maven central off that.

@sfc-gh-sparkes
Copy link

Excellent! Thanks.

@sfc-gh-sparkes
Copy link

@l46kok : https://github.com/Snowflake-Labs/cel-java/tree/smparkes/0.3.1
I can't create a branch in upstream so I can't create a PR to get this in but shouldn't be hard ...

@l46kok
Copy link
Collaborator

l46kok commented Feb 27, 2024

#259 -- you could likely expect this in maven central within O(days).

By any chance, would you mind sharing your use-case with us? We're interested in documenting best practices for CEL and would love any feedback. Be happy to follow up over email.

@sfc-gh-sparkes
Copy link

@l46kok: Thanks! More offline ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to pull Pulls the pending PR into Critique
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants