Return-Path: 
 <reviews-return-11417-apmail-aurora-reviews-archive=aurora.apache.org@aurora.apache.org>
X-Original-To: apmail-aurora-reviews-archive@minotaur.apache.org
Delivered-To: apmail-aurora-reviews-archive@minotaur.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 7303917BCE
	for <apmail-aurora-reviews-archive@minotaur.apache.org>;
 Fri, 11 Sep 2015 23:05:48 +0000 (UTC)
Received: (qmail 26157 invoked by uid 500); 11 Sep 2015 23:05:48 -0000
Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org
Received: (qmail 26112 invoked by uid 500); 11 Sep 2015 23:05:48 -0000
Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:reviews-help@aurora.apache.org>
List-Unsubscribe: <mailto:reviews-unsubscribe@aurora.apache.org>
List-Post: <mailto:reviews@aurora.apache.org>
List-Id: <reviews.aurora.apache.org>
Reply-To: reviews@aurora.apache.org
Delivered-To: mailing list reviews@aurora.apache.org
Received: (qmail 26085 invoked by uid 99); 11 Sep 2015 23:05:48 -0000
Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 11 Sep 2015 23:05:48 +0000
Received: from reviews.apache.org (localhost [127.0.0.1])
	by reviews.apache.org (Postfix) with ESMTP id 30B5C273124;
	Fri, 11 Sep 2015 23:05:47 +0000 (UTC)
Content-Type: multipart/alternative;
 boundary="===============5990641328652502507=="
MIME-Version: 1.0
Subject: Re: Review Request 38280: Restore build properties within Scheduler
 vars endpoint and snapshots
From: "Joe Smith" <yasumoto7@gmail.com>
To: "Bill Farner" <wfarner@apache.org>, "Maxim Khutornenko" <maxim@apache.org>
Cc: "Joe Smith" <yasumoto7@gmail.com>, "Zameer Manji" <zmanji@apache.org>,
 "Aurora" <reviews@aurora.apache.org>
Date: Fri, 11 Sep 2015 23:05:47 -0000
Message-ID: <20150911230547.12751.58815@reviews.apache.org>
X-ReviewBoard-URL: https://reviews.apache.org/
Auto-Submitted: auto-generated
Sender: "Joe Smith" <noreply@reviews.apache.org>
X-ReviewGroup: Aurora
X-Auto-Response-Suppress: DR, RN, OOF, AutoReply
X-ReviewRequest-URL: https://reviews.apache.org/r/38280/
X-Sender: "Joe Smith" <noreply@reviews.apache.org>
References: <20150911005010.1695.46115@reviews.apache.org>
In-Reply-To: <20150911005010.1695.46115@reviews.apache.org>
Reply-To: "Joe Smith" <yasumoto7@gmail.com>
X-ReviewRequest-Repository: aurora

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



> On Sept. 10, 2015, 5:50 p.m., Zameer Manji wrote:
> > .gitignore, line 14
> > <https://reviews.apache.org/r/38280/diff/1/?file=1067844#file1067844line14>
> >
> >     The gradle task seems to be writing `build.properties` to `dist/` is this needed?

Totally correct, thanks!


> On Sept. 10, 2015, 5:50 p.m., Zameer Manji wrote:
> > commons/src/main/java/org/apache/aurora/common/util/BuildInfo.java, line 39
> > <https://reviews.apache.org/r/38280/diff/1/?file=1067849#file1067849line39>
> >
> >     This should be Optional<Properties> and defaulted to Optional.absent().

Done.


> On Sept. 10, 2015, 5:50 p.m., Zameer Manji wrote:
> > commons/src/main/java/org/apache/aurora/common/util/BuildInfo.java, line 63
> > <https://reviews.apache.org/r/38280/diff/1/?file=1067849#file1067849line63>
> >
> >     We should call `fetchProperties` in the constructor that accepts a resource path. There is no need to delay scanning the classpath until the data is needed.

Bingo, thanks!


> On Sept. 10, 2015, 5:50 p.m., Zameer Manji wrote:
> > commons/src/main/java/org/apache/aurora/common/util/BuildInfo.java, line 85
> > <https://reviews.apache.org/r/38280/diff/1/?file=1067849#file1067849line85>
> >
> >     This should return Optional<Properties> since it is possible for the build.info to not be available.

Done.


> On Sept. 10, 2015, 5:50 p.m., Zameer Manji wrote:
> > build-support/generate-build-properties, line 29
> > <https://reviews.apache.org/r/38280/diff/1/?file=1067846#file1067846line29>
> >
> >     Is it possible to check if we are in a git repo via the git binary? As it stands this script is prone to breakage if we move it in the repo.

`git rev-parse` is what I should've looked for.


- Joe


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


On Sept. 10, 2015, 5:24 p.m., Joe Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38280/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2015, 5:24 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1473
>     https://issues.apache.org/jira/browse/AURORA-1473
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Restore build properties within Scheduler vars endpoint and snapshots
> 
> 
> Diffs
> -----
> 
>   .gitignore 6c37128b5d02b0e52eb467be3652a37b10d7bc06 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 670ba0850987308370e3c766048ad6ba246d9e29 
>   build-support/generate-build-properties PRE-CREATION 
>   build.gradle 9c78aff101793b25e4c1196c170eaf282f73a9bf 
>   commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java 3959ce3d688dd50399185925d91f0014fc1c43f9 
>   commons/src/main/java/org/apache/aurora/common/util/BuildInfo.java PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/util/testing/FakeBuildInfo.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java d76596c7422786e58b5a8aa79f324911cfd29b25 
>   src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 50838de8708d78fc0bd7ee672b7c7ba02dfcd505 
>   src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java cc9c066556385c073962903691c037b0c07cc94c 
>   src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 6e032a6cef86e5f408bfc0d853a52c7f898d0db8 
> 
> Diff: https://reviews.apache.org/r/38280/diff/
> 
> 
> Testing
> -------
> 
> `./gradlew build -Pq`
> 
> 
> Thanks,
> 
> Joe Smith
> 
>


--===============5990641328652502507==--