Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.apache.org
Content-Type: multipart/alternative;
 boundary="===============8164646183630583713=="
MIME-Version: 1.0
Subject: Re: Review Request 39150: Converting to Range in ConfigGroup thrift.
From: "Kevin Sweeney" <kevints@apache.org>
To: "Joshua Cohen" <jcohen@apache.org>, "Bill Farner" <wfarner@apache.org>
Cc: "David McLaughlin" <david@dmclaughlin.com>,
 "Kevin Sweeney" <kevints@apache.org>, "Maxim Khutornenko" <maxim@apache.org>,
 "Aurora" <reviews@aurora.apache.org>
Date: Fri, 09 Oct 2015 17:23:31 -0000
Message-ID: <20151009172331.31883.98945@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Kevin Sweeney" <noreply@reviews.apache.org>
References: <20151009171247.16483.19792@reviews.apache.org>
In-Reply-To: <20151009171247.16483.19792@reviews.apache.org>
Reply-To: "Kevin Sweeney" <kevints@apache.org>

--===============8164646183630583713==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Oct. 9, 2015, 10:12 a.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?

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.


- Kevin


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


On Oct. 8, 2015, 6:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39150/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2015, 6:32 p.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
> 
>


--===============8164646183630583713==--