aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joe Smith" <yasumo...@gmail.com>
Subject Re: Review Request 38280: Restore build properties within Scheduler vars endpoint and snapshots
Date Fri, 11 Sep 2015 23:05:47 GMT


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


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