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

Requesting new outputs for OpenVDB #1313

Merged
merged 3 commits into from
Jan 14, 2025

Conversation

tetov
Copy link
Contributor

@tetov tetov commented Jan 14, 2025

Checklist:

  • I want to add a package output to a feedstock:
    • Pinged the relevant feedstock team(s): @conda-forge/openvdb
    • Added a small description of why the output is being added.

Description

I'm adding NanoVDB from the OpenVDB project well as packaging tools for OpenVDB and NanoVDB.

@tetov tetov requested a review from a team as a code owner January 14, 2025 16:20
@tetov tetov changed the title request new outputs for openvdb Requesting new outputs for OpenVDB Jan 14, 2025
Comment on lines 3 to 4
- openvdb: openvdb*
- openvdb: nanovdb*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- openvdb: openvdb*
- openvdb: nanovdb*
- openvdb: openvdb-*
- openvdb: nanovdb-*

Copy link
Contributor Author

@tetov tetov Jan 14, 2025

Choose a reason for hiding this comment

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

Thank you for reviewing!

I updated like you suggest, but added nanovdb without glob as well.

I have used underscore in the output names in the recipe. Would dashes be preferable?

action: add_feedstock_output
feedstock_to_output_mapping:
  - openvdb: openvdb-*
  - openvdb: nanovdb
  - openvdb: nanovdb-*

Copy link
Member

Choose a reason for hiding this comment

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

Either a dash or underscore is fine. I find dashes aesthetically more please but this is a personal preference.

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 took a quick look at the referenced example packages with multiple outputs and they all used -, so I'll follow that.

Please merge if you are happy with the req and I'll push with dashes in my PR :).

Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

we need to keep the namespace cleaner.

@beckermr beckermr enabled auto-merge (squash) January 14, 2025 17:17
@beckermr beckermr merged commit 85d6a66 into conda-forge:main Jan 14, 2025
1 check passed
@tetov tetov deleted the add_openvdb_sub_pkgs branch January 15, 2025 09:28
tetov added a commit to conda-forge/openvdb-feedstock that referenced this pull request Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants