I had some compile errors because Documentation::new() has changed signature (it now has three required arguments) after this PR from @comphead

Thanks @alamb please let me know where are the compile errors? Is it a downstream project compilation?

what probably we can do in a short run is to have a migration document and describe migration points.
Especially it is crucial for PRs with

• api_change
• introducing deprecated methods
• removing obsolete methods

So we need to introduce a single file doc with migration and its gonna part of review to make sure migration steps are added if any of 3 conditions above are met

WDYT @alamb

One possible scenario is have a file migration.md with following structure

Release 43.0.0
- steps to migrate feature1
- steps to migrate feature2

Release 42.0.0
- steps to migrate feature1
- steps to migrate feature2

Without the migration every migration the user never knows what to expect unless he starts to migrate

Thanks @alamb please let me know where are the compile errors? Is it a downstream project compilation?

It was example on the issue filed by @kylebarron on #13762

Basically

static DOC: OnceLock<Documentation> = OnceLock::new();
impl ScalarUDFImpl for UnionExample {
...
    fn documentation(&self) -> Option<&Documentation> {
        Some(DOC.get_or_init(|| Documentation::builder().build().unwrap()))
    }
...
}

This compiles in 43.0.0 but on 44.0.0 you get an error about "DocumentationBuilder requires 3 arguments" or something like that. Then I followed the source and found I needed to provide &DocSection, etc....

I could have figured it out but I was focused on something else -- it would have been much nicer if it had been a warning / deprecation message that I could choose to ignore for the time being and come back to when I had time. It also would have been nice if it pointed me at what to do (aka use the nice new doc macro)

what probably we can do in a short run is to have a migration document and describe migration points. Especially it is crucial for PRs with

• api_change
• introducing deprecated methods
• removing obsolete methods

So we need to introduce a single file doc with migration and its gonna part of review to make sure migration steps are added if any of 3 conditions above are met

WDYT @alamb

I think the file is a great idea ❤️

I think it would be great if we could pull off adding this to the reviews. I worry about being able to get contributors to write such a guide (it may be too big of a burden). Though maybe once we have some good examples it would be ok

I still believe that #[deprecatated] for a few releases is a much better pattern, FWIW as it gives downstream users more flexibility.

(I can make a PR with a proposed change, I just wanted to socialize it a bit first)

I'll create the PR soon with the sample file and some doc how to use it and how review process will be changed and we can through it

I'll create the PR soon with the sample file and some doc how to use it and how review process will be changed and we can through it

Would it be ok to do renaming functions/ deprecation (in addition)?

Here is a proposed PR:

• Restore DocBuilder::new() to avoid breaking API change #13870