Re: Deprecate DynamicRegionFactory

+1 to remove it On Tue, Oct 11, 2016 at 9:33 PM, John Blum wrote: > +1 - forget deprecated; delete it. > > On Tue, Oct 11, 2016 at 8:22 PM, Udo Kohlmeyer > wrote: > > > +1 - I see no real benefit for this. > > > > > > > > On 12/10/16 1:10 pm, William Markito wrote: > > > >> +1 - We always disc

Re: Deprecate DynamicRegionFactory

+1 Le 10/12/2016 à 6:36 AM, Jens Deppe a écrit : +1 to remove it On Tue, Oct 11, 2016 at 9:33 PM, John Blum wrote: +1 - forget deprecated; delete it. On Tue, Oct 11, 2016 at 8:22 PM, Udo Kohlmeyer wrote: +1 - I see no real benefit for this. On 12/10/16 1:10 pm, William Markito wrote:

[GitHub] incubator-geode pull request #256: Feature/geode 1532

Github user jaredjstewart commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/256#discussion_r83038180 --- Diff: geode-pulse/src/main/webapp/WEB-INF/spring-security.xml --- @@ -20,15 +20,14 @@ xmlns:context="http://www.springframework.org/

[GitHub] incubator-geode pull request #256: Feature/geode 1532

Github user jaredjstewart commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/256#discussion_r83038809 --- Diff: geode-pulse/build.gradle --- @@ -68,6 +68,11 @@ dependencies { testCompile project(':geode-core') testCompile files(proj

Re: Review Request 52742: GEODE-1979: refactor tests using LocatorServerConfigurationRule

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52742/#review152333 --- Ship it! Ship It! - Kirk Lund On Oct. 12, 2016, 1:11 a.m., J

Re: Review Request 52751: GEODE-1986: correctly set the flag indicating if cluster configuration service is running or not on a locator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52751/#review152334 --- Fix it, then Ship it! Fix use of curlies and Ship It! geode-c

[GitHub] incubator-geode issue #257: Feature/geode 1952

Github user davebarnes97 commented on the issue: https://github.com/apache/incubator-geode/pull/257 Nice work! Two comments: 1. As mentioned, I get one orphaned file during the bookbinder build: pdf_header.html. 2. README.md sugggestions: a. "Bookbinder is a gem" -> "Bookbi

Re: Review Request 52742: GEODE-1979: refactor tests using LocatorServerConfigurationRule

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52742/#review152342 --- Ship it! Ship It! - Kevin Duling On Oct. 11, 2016, 6:11 p.m.

Re: Review Request 52751: GEODE-1986: correctly set the flag indicating if cluster configuration service is running or not on a locator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52751/#review152345 --- Ship it! Ship It! - Kevin Duling On Oct. 11, 2016, 7:21 p.m.

Coding practices/standards

I would like to advocate for adding a Checkstyle or Spotless gradle task to our build process to ensure that all code checked in meets the formatting standards described on the wiki

Re: Coding practices/standards

+1 This might be a good time to reformat the code since I don't think there are too many long lived feature branches outstanding. -Dan On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart wrote: > I would like to advocate for adding a Checkstyle sourceforge.net/> or Spotless <

Re: Coding practices/standards

+1 On Wed, Oct 12, 2016 at 10:06 AM Dan Smith wrote: > +1 > > This might be a good time to reformat the code since I don't think there > are too many long lived feature branches outstanding. > > -Dan > > On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart > wrote: > > > I would like to advocate for

[GitHub] incubator-geode pull request #256: Feature/geode 1532

Github user asfgit closed the pull request at: https://github.com/apache/incubator-geode/pull/256 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the featur

Re: Coding practices/standards

+1 When a pull request is submitted some type of build is automatically done to make sure the code compiles. Would this new "code check" also be done in that automatic check? On Wed, Oct 12, 2016 at 10:07 AM, Nabarun Nag wrote: > +1 > > On Wed, Oct 12, 2016 at 10:06 AM Dan Smith wrote: > > > +

Re: Coding practices/standards

I know I've been guilty of submitting code for review that hasn't matched the code style in the past. As a result, I've checked the "reformat code" box on the 'commit' dialog in IDEA, pointing to the IntelliJ xml formatter. It's frustrating to me that this sort of thing is still an issue in softw

Re: Coding practices/standards

Hi Jared, The Checkstyle plugin is already added to the Gradle build. By default it is turned off, however, if you pass the property '-PstaticAnalysis' to the command-line, it will run CheckStyle and FindBugs. It was setup with only basic configuration, so the small amount of work would be to add

Review Request 52789: GEODE-1978: Slowing down the receivers

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52789/ --- Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian zh

Re: Coding practices/standards

Source code with a consistent look-and-feel makes it easier for people to join the project community and contribute. Let’s continue to keep reformatting commits separate from logic changes—otherwise it’s too hard to review. Anthony > On Oct 12, 2016, at 10:06 AM, Dan Smith wrote: > > +1 > >

Re: Review Request 52789: GEODE-1978: Slowing down the receivers

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52789/#review152352 --- Ship it! Ship It! - Dan Smith On Oct. 12, 2016, 5:21 p.m., n

Review Request 52793: GEODE-1353: Added listeners to slow down the receiver.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52793/ --- Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian zh

Re: Coding practices/standards

I like the idea of doing this but I don't think Checkstyle should be enabled until all of the code is reformatted. Also, last time I checked there was still a problem with the IntelliJ auto-format settings. It still used wildcard imports, which I believe we don't allow. I've manually changed

Re: Coding practices/standards

It would certainly be necessary to make sure that the code style to be enforced is sensible, e.g. doe not use wildcard imports. We would also want to make one large commit to format all existing code before turning this on. Mark - Thank you for the information about the existing setup. Mark, D

Review Request 52796: GEODE-1981: Synchronizing the ResultCollector when adding results

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52796/ --- Review request for geode, Barry Oglesby and nabarun nag. Repository: geode De

Re: Coding practices/standards

If Travis CI is only run on pull requests then that is not enough because committers do not submit pull requests. Having it run during the gradle build or precheckin target is also needed. In addition to that I also wanted PRs to be checked. On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart wrote:

Re: Coding practices/standards

+1 to running during the precheckin target as well as Travis CI On Oct 12, 2016 11:20 AM, "Darrel Schneider" wrote: > If Travis CI is only run on pull requests then that is not enough because > committers do not submit pull requests. Having it run during the gradle > build or precheckin target i

Re: Coding practices/standards

Seems like it should run as part of the build target. The only reason to make it part of precheckin is if it takes a long time, otherwise people should get fast feedback they need to change their code before they push. -Dan On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart wrote: > +1 to running

Re: Coding practices/standards

Dan, There is some extra amount of time, 5-10 minutes extra for the entire project (depending on your CPU). I think the real issue to adding it to the precheckin target and have it be 'effective' is it needs to run successfully, otherwise it would turn into noise most of the time I think. We need

Re: Coding practices/standards

+1 - adding checkstyle to precheckin. If the developer uses the provided templates ( eclipse + intellij) then most of the formatting issues should be handled before precheckin. Also, if a developer has a questionable coding style, that should lessen as that developer will have resolve the issu

[GitHub] incubator-geode pull request #257: Feature/geode 1952

Github user joeymcallister closed the pull request at: https://github.com/apache/incubator-geode/pull/257 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if th

[GitHub] incubator-geode issue #257: Feature/geode 1952

Github user joeymcallister commented on the issue: https://github.com/apache/incubator-geode/pull/257 Thanks, Dave. Closing this PR to make the changes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project doe

Re: Review Request 52796: GEODE-1981: Synchronizing the ResultCollector when adding results

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52796/#review152387 --- Based on feedback from Naba, I'll rework this to just wrap the res

Review Request 52805: GEODE-1981: Wrapping user ResultCollector in synchronized wrapper

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52805/ --- Review request for geode, Barry Oglesby and nabarun nag. Repository: geode De

Re: Coding practices/standards

I like Dan's idea of catching formatting issues earlier but I think adding 5-10 minutes to "build" would be too much. Currently when I'm trying to do a quick build I use -xjavadoc. I'd probably do the same for this target if it was part of build until I'm ready to do a precheckin. Mark, wouldn't r

Re: Review Request 52805: GEODE-1981: Wrapping user ResultCollector in synchronized wrapper

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52805/#review152401 --- Ship it! Ship It! - nabarun nag On Oct. 12, 2016, 8:58 p.m.,

[GitHub] incubator-geode pull request #258: Feature/geode 1952 2

GitHub user joeymcallister opened a pull request: https://github.com/apache/incubator-geode/pull/258 Feature/geode 1952 2 This merges feature/GEODE-1952, which was branched from the docs donation staging branch, into feature/GEODE-1952-2, which was branched from develop.

Re: Coding practices/standards

I did some investigation this afternoon on using the Gradle Spotless plugin rather than Checkstyle. Whereas Checkstyle has its own formatting template syntax, Spotless can import our existing Eclipse formatter template. The plugin adds tasks for “spotlessCheck” (which tells you whether or not

Re: Coding practices/standards

If you want to try it out, I pushed a branch to my Geode repo that contains this change: https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin > On Oct 12, 2016, at 2:27 PM, Darrel Schneider wrote: > > I like

[GitHub] incubator-geode issue #258: Feature/geode 1952 2

Github user davebarnes97 commented on the issue: https://github.com/apache/incubator-geode/pull/258 +1 Tested the doc build, succeeded without errors. (Disclaimer - bookbinder was already resident on my system) --- If your project is set up for it, you can reply to this email

[GitHub] incubator-geode issue #258: Feature/geode 1952 2

Github user metatype commented on the issue: https://github.com/apache/incubator-geode/pull/258 I think the the CONTRIBUTE.md file needs some updates: - Remove everything before "Working with Markdown Files" - Replace references to 'Project Geode' with 'Apache Geode' Th

Re: Coding practices/standards

Agree with Mark, this has to work with 0 errors before it would be useful in precheckin. I think I could live with an additional 17 seconds most of the time for running the spotlessCheck as suggested. Jared, Is that 17 seconds running locally on your laptop or on a more capable machine? Ken >

Re: Coding practices/standards

This is running locally on my laptop. Since Spotless is only doing code formatting and not any other static analysis, it already has 0 errors. (Other than, of course, formatting not consistent with the template.) > On Oct 12, 2016, at 4:11 PM, Kenneth Howe wrote: > > Agree with Mark, this ha

Review Request 52812: GEODE-1999: Fix offheap memory leak when exception is thrown during basicDestroy call to remove GatewaySenderEventImpl from the sender queue

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52812/ --- Review request for geode and Darrel Schneider. Bugs: GOEDE-1999 https://iss

Re: Review Request 52812: GEODE-1999: Fix offheap memory leak when exception is thrown during basicDestroy call to remove GatewaySenderEventImpl from the sender queue

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52812/#review152422 --- Ship it! Ship It! - Darrel Schneider On Oct. 12, 2016, 4:32

[VIRTUAL] Geode Clubhouse Weds, 10/19 9AM Pacific - New Security Framework for Apache Geode

Dear Geode Community, Ok, its back to APIs this month as we explore the new security framework with Jinmei Liao this next Weds, October 19, at 9AM Pacific. Add to calendar

[GitHub] incubator-geode pull request #258: Feature/geode 1952 2

Github user joeymcallister closed the pull request at: https://github.com/apache/incubator-geode/pull/258 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if th

[GitHub] incubator-geode pull request #259: Feature/geode 1952 2

GitHub user joeymcallister opened a pull request: https://github.com/apache/incubator-geode/pull/259 Feature/geode 1952 2 This merges feature/GEODE-1952, which was branched from the docs donation staging branch, into feature/GEODE-1952-2, which was branched from develop.

Re: Coding practices/standards

This sounds really good to me as well. +1 On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart wrote: > This is running locally on my laptop. Since Spotless is only doing code > formatting and not any other static analysis, it already has 0 errors. > (Other than, of course, formatting not consistent