> On Oct. 9, 2015, 5:12 p.m., Kevin Sweeney wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 351
> > <https://reviews.apache.org/r/39150/diff/2/?file=1093460#file1093460line351>
> >
> >     Please rename this to deprecatedInstanceIds.
> 
> David McLaughlin wrote:
>     -1 to this deprecation technique.
> 
> Maxim Khutornenko wrote:
>     I thought we decided to not follow this approach and rely on 
> announcements and deprecation tickets instead?
> 
> Kevin Sweeney wrote:
>     Can you elaborate on the rationale behind your -1?
>     
>     Renaming this field does not alter binary compatibility in any way and 
> provides feedback to the API user that the field is slated for removal (in 
> the form of a compiler or unit test break, similar to the `@Deprecated` 
> annotation). A comment here is very likely to be missed. We have used this 
> technique in the past.

IMO, renaming fields does not necessarily improve our deprecation story but 
creates upgrade troubles twice more often: first on DEPRECATED renaming and 
second on field removal.

The @Deprecated annotation != field renaming as it does not break on upgrade 
(unless build env set to break on warnings, where it still can be suppressed 
without code changes).

We have stopped following DEPRECATED renaming awhile ago due to the above 
reasons.


- Maxim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39150/#review102083
-----------------------------------------------------------


On Oct. 9, 2015, 1:32 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39150/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 1:32 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Addressing a long standing TODO. Also some cleanup of unused util functions.
> 
> 
> Diffs
> -----
> 
>   NEWS 2edcea61fafea09b08faf03af4335ca82629d46d 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> a04e644453bfecde44ec7b51b53f42dc82e90c96 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5c1bdb4aeab285c46475a54bd13aeb780541fa08 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 13b5c222a0d7b9dd347990e6c09aac09ee566315 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  fd07365faa5fe516680600ed774d35c564948b9b 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 85ae8ce9e51d1785e32ba3f2e4f31f35ec6f178d 
>   src/main/resources/scheduler/assets/js/services.js 
> b7699fe91d79f9a8141c8368da91443684b6994b 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  64cbd5b58d9254bb741e20e47165732e52569f70 
> 
> Diff: https://reviews.apache.org/r/39150/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual UI testing
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to