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

Use pdl-compiler directly #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marshallpierce
Copy link
Contributor

If the pdlc binary is not available, use pdl_compiler as a library instead.

Also do some misc tidying of the build script, since it looks like the output path was being derived in different ways with the same result.

@marshallpierce
Copy link
Contributor Author

May not be needed after google/pdl#61.

- name: Install dependencies
run: |
cargo install pdl-compiler --version 0.2.2
cargo install pdl-compiler --version 0.2.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming it's desirable for pdl-compiler to be up to date -- if you'd rather stick with 0.2.2 that's fine too of course


// Find the pdl tool. Expecting it at CARGO_HOME/bin
let pdl = match env::var("CARGO_HOME") {
let pdlc = match env::var("CARGO_HOME") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it even desirable to maintain this code path? If someone is using Cargo vs the Blaze build, when would they want to use the external pdlc? Or perhaps should it be present to allow hypothetical debugging of issues specific to a pdlc binary, but only used if an env var is set?

@marshallpierce marshallpierce marked this pull request as ready for review February 26, 2024 16:52
@@ -26,9 +26,13 @@ jobs:
with:
submodules: recursive

- name: Check Cargo build
run: |
cargo test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

might as well check that the cargo build works 🤷

If the `pdlc` binary is not available, use `pdl_compiler` as a library
instead.

Also do some misc tidying of the build script, since it looks like the
output path was being derived in different ways with the same result.
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.

1 participant