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

doc: Add link to contributor's guide for new functions within the src #7240

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

2010YOUY01
Copy link
Contributor

Which issue does this PR close?

NA

Rationale for this change

I've noticed it's easy to forget to add SQL reference documentation when adding new functions.
Automate this task seems hard to do and maintain, so I think at least we can remind contributors to do so somewhere in the src, function ENUM declaration should be a good place for such README

What changes are included in this PR?

  1. Update the contributor's guide for adding functions with sqllogictest and SQL reference doc
  2. Place the link close to the function ENUM declaration

Are these changes tested?

The doc has been built locally and all links have been tested

Are there any user-facing changes?

No

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Aug 9, 2023
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @2010YOUY01 -- this is a very nice improvement

@@ -25,6 +25,8 @@ use std::{fmt, str::FromStr};
use strum_macros::EnumIter;

/// Enum of all built-in aggregate functions
// Contributor's guide for adding new aggregate functions
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@alamb alamb merged commit d8692b1 into apache:main Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logical-expr Logical plan and expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants