Update documentation guidelines for contribution content #13703
Conversation
github-actions
bot
added
the
documentation
|
Rather than state some hard rule, I went with the approach of guidance on what would need more / less discussion prior to acceptance. I felt this would give us flexibility but still given potential contributors guidance |
| 2. Test coverage for existing features | ||
| 3. Documentation improvements / examples | ||
| 4. Performance improvements to existing features (with benchmarks) | ||
| 5. "Small" functional improvements to existing features |
There was a problem hiding this comment.
| 5. "Small" functional improvements to existing features | |
| 5. "Small" functional improvements to existing features |
Non breaking improvements?
There was a problem hiding this comment.
That is a good qualification -- non breaking changes are certainly more likely to get approved in my experience. I tried to clarify
| 5. "Small" functional improvements to existing features | ||
| 6. Additional APIs for extending DataFusion's capabilities | ||
|
|
||
| Contributions that likely require discussion prior to acceptance include: |
There was a problem hiding this comment.
We probably should note the preferred way to discuss, GH Discussion/ASF slack, etc
| Contributions that likely require discussion prior to acceptance include: | ||
|
|
||
| 1. New functionality that is part of the "standard sql" | ||
| 2. New functions that aren't part of the "standard sql" |
There was a problem hiding this comment.
Not sure I'm following, 🤦 looks like anything related to "standard sql" requires discussion?
There was a problem hiding this comment.
I changed the wording like this so nothing was "required" but to give a hint that major new features are likely to involve more discussion
Contributions that will likely involve more discussion (see Discussing New
Features above) prior to acceptance include:
1. Major new functionality (even if it is part of the "standard sql")
2. New functions, especially if they aren't part of "standard sql"
3. New data sources (e.g. support for Apache ORC)
| 1. Bug fixes for existing features | ||
| 2. Test coverage for existing features | ||
| 3. Documentation improvements / examples | ||
| 4. Performance improvements to existing features (with benchmarks) |
|
BTW We should strive to make datafusion easier to extend. This is not only about particular APIs but also about internal design. We may be principled or not about what kind of SQL variant we provide out of the box, but we should assume extending systems provide something else (eg Ballista speaks Spark SQL, which is different than Datafusion SQL). This is obviously a teaser to longer discussion (#12723). I approved this PR based on my understand that the wording proposed here doesn't preclude -- and actually encourages -- ground work features serving the extensibility story (the issue linked above, #12604, etc.) |
Co-authored-by: Piotr Findeisen <[email protected]> Co-authored-by: Oleks V <[email protected]>
I 100% agree
Yes, this was my intention and I think it fits directly with the stated design goals (aka customizable everything):
|
|
Should we let it hang for day-two to get other reviews? |
Yes I think we should leave this open for several more days to make sure anyone who wants a chance to comment can do so. Obviously we can always update the wordiing with subsequent PRs too, but I don't think there is any reason to rush this one in |
Co-authored-by: Piotr Findeisen <[email protected]>
|
LGTM |
|
Thanks for all the comments so far. I plan to merge this tomorrow, Monday Dec 16 unless anyone else would like time to comment |
|
🚀 📖 |
Which issue does this PR close?
Rationale for this change
Given recent discussions on improving DataFusion stability (see #13648) I think it is time to document the criteria to add new features to DataFusion to help ensure everyone's expectations are aligned
What changes are included in this PR?
Are these changes tested?
CI
Are there any user-facing changes?