Re: [DISCUSS] Enforce tighter control on API related changes

2020-01-14 Thread Lin Yuan
Sheng, I will provide more detail on the GitHub issue. The "API Change" labeling for PRs sounds like a good solution to keep consistent API design across MXNet. I guess we can close the discussion on this topic now. Best, Lin On Tue, Jan 14, 2020 at 6:02 PM Sheng Zha wrote: > > 2)

Re: [DISCUSS] Enforce tighter control on API related changes

2020-01-14 Thread Sheng Zha
> 2) Regarding issue #17292, it was not broken by 4ed14e2 but an C API > change in in https://github.com/apache/incubator-mxnet/pull/17128. The > later commit 4ed14e2 was trying to fix this API change but it did not seem > to work yet. None of the existing C API was changed in #17128. #17128 had

Re: [DISCUSS] Enforce tighter control on API related changes

2020-01-14 Thread Lin Yuan
Hi Sheng, Thanks for your reply. 1) Adding a "API Change" label is a good way to flag PRs with API change. It would be great if we could make this labeling automatic with some hook in API related modules, so we don't miss them in PRs. 2) Regarding issue #17292, it was not broken by 4ed14e2 but

Re: [DISCUSS] Enforce tighter control on API related changes

2020-01-13 Thread Sheng Zha
Hi Lin, Thanks for the suggestions. With respect to your proposal: > (2) Any PR that contains API change should clearly state this in PR title. > Otherwise, committer can reject the PR I agree that PRs with API changes should be made more prominent. Another mechanism that has already been

Re: [DISCUSS] Enforce tighter control on API related changes

2020-01-13 Thread Tianqi Chen
I wonder if it is possible to just use the RFC mechanism for most API change discussions. Note that while API compatibility is important, having a clear RFC mechanism would likely strike a balance between the need to evolve APIs (e.g. 2.0) and stability of the project TQ On Mon, Jan 13, 2020 at

[DISCUSS] Enforce tighter control on API related changes

2020-01-13 Thread Lin Yuan
Dear Community, Recently, there were some changes to C APIs that broke another downstream project Horovod: https://github.com/apache/incubator-mxnet/issues/17292. Since we do not have integration tests for downstream project, it becomes critical for us to update APIs with extreme caution. I