Skip to content

Conversation

@thomasqueirozb
Copy link
Contributor

@thomasqueirozb thomasqueirozb commented Nov 6, 2025

Summary

This macro prevents functions from being exported but not included in the stdlib. Should make maintenence better and prevent issues like #1553 from happening

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

How did you test this PR?

I verified # of imports before (200) and lines inside the macro (199). It is 199 since pub use get_timezone_name::get_name_for_timezone isn't and shouldn't be included inside all().

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on
    our guidelines.
  • No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

  • Our CONTRIBUTING.md is a good starting place.
  • If this PR introduces changes to LICENSE-3rdparty.csv, please
    run dd-rust-license-tool write and commit the changes. More details here.
  • For new VRL functions, please also create a sibling PR in Vector to document the new function.

References

@thomasqueirozb thomasqueirozb added the no-changelog Changes in this PR do not need user-facing explanations in the release changelog label Nov 6, 2025
@thomasqueirozb thomasqueirozb marked this pull request as ready for review November 6, 2025 15:35
@thomasqueirozb thomasqueirozb requested a review from a team as a code owner November 6, 2025 15:35
Copy link
Member

@pront pront left a comment

Choose a reason for hiding this comment

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

This is a pretty good improvement over what we have today.

@pront pront added this pull request to the merge queue Nov 7, 2025
Merged via the queue into main with commit f5b33e6 Nov 7, 2025
16 checks passed
@pront pront deleted the vrl-stdlib-macro branch November 7, 2025 17:42
Copy link
Member

@pront pront left a comment

Choose a reason for hiding this comment

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

Note for posterity: This PR is an improvement (since now the contributor needs to add the new function name in one place only) but not a real fix. One fix would be to add a test that detects missing function names based on stdlib contents. There might be better alternatives, e.g. generate this list in a build.rs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Changes in this PR do not need user-facing explanations in the release changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants