aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 39150: Converting to Range in ConfigGroup thrift.
Date Fri, 09 Oct 2015 17:39:31 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message