Re: Return types form methods on Cache

2017-09-18 Thread Jacob Barrett


> On Sep 18, 2017, at 7:34 PM, Kirk Lund  wrote:
> 
> I would vote +1 for a more attractive, professional and user-friendly API.
> I'm not sure if there's a perf or memory-usage reason for using
> "std::shared_ptr" to types instead of returning values, but the end
> result does not look like a professional API to me.

There really isn’t, especially if you look at what we did dirty 
CacheFactory::getCache by returning a value that can be moved into the heap and 
a shared point of one wants but not being forced into it. RVO tricks can event 
make that move a noop.

auto r = cache.getRegion(...);
Where decltype(r) == Region
and
auto rp = std::make_shared(cache->getRegion());
Where decltype(rp) == shared_ptr

Would both be valid.




Re: Return types form methods on Cache

2017-09-18 Thread Kirk Lund
I would vote +1 for a more attractive, professional and user-friendly API.
I'm not sure if there's a perf or memory-usage reason for using
"std::shared_ptr" to types instead of returning values, but the end
result does not look like a professional API to me.

I would favor a more user-friendly API. If there's a performance or memory
constraint to returning value instead of shared ptr then applying such a
pattern to getRegion is overkill at the wrong level of abstraction -- the
user can easily keep the return value in a var instead of repeating the
Cache::getRegion call.

On Mon, Sep 18, 2017 at 3:23 PM, Jacob Barrett  wrote:

> So, now that we have selected to use the value model for
> CacheFactory::create()/Cache, we need to look at what the methods on Cache
> return.
>
> Starting with Cache::createRegionFactory method which returns
> std::shared_ptr. I think it is very clear that this should
> be a value type.
>
> RegionFactory Cache::createRegionFactory(...) const;
>
>
> Looking then at Cache::getRegion things are more interesting. It currently
> returns std::shared_ptr. Internally we keep a set of these
> shared_ptrs in a map. Does it make sense to keep returning these
> shared_ptrs or should we again switch to a value model? It would be great
> for consistency on the public API to go the value model way even if
> initially the value simply holds a shared_ptr to the internal region impl.
>
> Thoughts?
>


Passed: apache/geode#3900 (develop - 280b5d7)

2017-09-18 Thread Travis CI
Build Update for apache/geode
-

Build: #3900
Status: Passed

Duration: 12 minutes and 12 seconds
Commit: 280b5d7 (develop)
Author: Amey Barve
Message: GEODE-2719: corrected GEODE versions to 1.3.0 instead of 1.2.2 for the
respective deprecated API javadocs.

View the changeset: 
https://github.com/apache/geode/compare/7d4988aec34a...280b5d7bf988

View the full build log and details: 
https://travis-ci.org/apache/geode/builds/277085630?utm_source=email_medium=notification

--

You can configure recipients for build notifications in your .travis.yml file. 
See https://docs.travis-ci.com/user/notifications



Re: [VOTE] Apache Geode release - v1.2.1 RC4

2017-09-18 Thread Dick Cavender
+1

Downloaded source and binary dists.
Built and tested sources on Ubuntu against JDK 1.8.0_111
Ran gfsh full version followed by 5 minute tutorial on binary dist.
Review LICENSE file and copyrights.

-Dick

On Sat, Sep 16, 2017 at 8:01 AM, Anthony Baker  wrote:

> This is the fourth release candidate for Apache Geode, version 1.2.1.
> Thanks to all the community members for their contributions to this
> release!
>
> *** Please download, test and vote by Wednesday, September 20, 0800 hrs
> US Pacific. ***
>
> It fixes the following issues:
>  https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> projectId=12318420=12341124
>
> Note that we are voting upon the source tags:  rel/v1.2.1.RC4
>  https://github.com/apache/geode/tree/rel/v1.2.1.RC4
>  https://github.com/apache/geode-examples/tree/rel/v1.2.1.RC4
>
> Commit ID:
>  0b881b515eb1dcea974f0f5c1b40da03d42af9cf (geode)
>  5d034de588a43cdefb8fbb3a6259579785768340 (geode-examples)
>
> Source and binary files:
>  https://dist.apache.org/repos/dist/dev/geode/1.2.1.RC4
>
> Maven staging repo:
>  https://repository.apache.org/content/repositories/orgapachegeode-1027
>
> Geode's KEYS file containing PGP keys we use to sign the release:
>  https://github.com/apache/geode/blob/develop/KEYS
>
> pub  4096R/C72CFB64 2015-10-01
>  Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64
>
>
> Anthony
>


Re: [VOTE] Apache Geode release - v1.2.1 RC4

2017-09-18 Thread Karen Miller
+1

Ran the 1.2.1 RC4 partitioned geode-example against Geode 1.2.1 RC4.  So,
built Geode from
source, built one of the examples, ran gfsh commands to start locator and
servers as well as ran
a query, and did some puts and gets via API.

Also checked that the manual builds and contains 1.2.1 changes.


On Mon, Sep 18, 2017 at 3:10 PM, William Markito Oliveira <
mark...@apache.org> wrote:

> +1
>
> Checked:
> - Basic operations from command line
> - Checksums match
> - Release build green (blue actually)  (
> https://builds.apache.org/job/Geode-release/lastBuild/)
>
>
>
> On Mon, Sep 18, 2017 at 3:57 PM, Dan Smith  wrote:
>
> > +1
> >
> > The jenkins release build was green and release check passed as well -
> > https://github.com/upthewaterspout/geode-release-check
> >
> > -Dan
> >
> > On Sat, Sep 16, 2017 at 8:01 AM, Anthony Baker 
> wrote:
> >
> > > This is the fourth release candidate for Apache Geode, version 1.2.1.
> > > Thanks to all the community members for their contributions to this
> > > release!
> > >
> > > *** Please download, test and vote by Wednesday, September 20, 0800 hrs
> > > US Pacific. ***
> > >
> > > It fixes the following issues:
> > >  https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> > > projectId=12318420=12341124
> > >
> > > Note that we are voting upon the source tags:  rel/v1.2.1.RC4
> > >  https://github.com/apache/geode/tree/rel/v1.2.1.RC4
> > >  https://github.com/apache/geode-examples/tree/rel/v1.2.1.RC4
> > >
> > > Commit ID:
> > >  0b881b515eb1dcea974f0f5c1b40da03d42af9cf (geode)
> > >  5d034de588a43cdefb8fbb3a6259579785768340 (geode-examples)
> > >
> > > Source and binary files:
> > >  https://dist.apache.org/repos/dist/dev/geode/1.2.1.RC4
> > >
> > > Maven staging repo:
> > >  https://repository.apache.org/content/repositories/
> orgapachegeode-1027
> > >
> > > Geode's KEYS file containing PGP keys we use to sign the release:
> > >  https://github.com/apache/geode/blob/develop/KEYS
> > >
> > > pub  4096R/C72CFB64 2015-10-01
> > >  Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64
> > >
> > >
> > > Anthony
> > >
> >
>


[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #682 was SUCCESSFUL (with 2042 tests). Change made by Mark Paluch.

2017-09-18 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #682 was successful.
---
Scheduled with changes by Mark Paluch.
2044 tests in total.

https://build.spring.io/browse/SGF-NAG-682/




--
Code Changes
--
Mark Paluch (d52bea06c19753ff2821604318ae231a5a983e2e):

>DATAGEODE-42 - Upgrade to OpenWebBeans 2.0.1



--
This message is automatically generated by Atlassian Bamboo

Return types form methods on Cache

2017-09-18 Thread Jacob Barrett
So, now that we have selected to use the value model for
CacheFactory::create()/Cache, we need to look at what the methods on Cache
return.

Starting with Cache::createRegionFactory method which returns
std::shared_ptr. I think it is very clear that this should
be a value type.

RegionFactory Cache::createRegionFactory(...) const;


Looking then at Cache::getRegion things are more interesting. It currently
returns std::shared_ptr. Internally we keep a set of these
shared_ptrs in a map. Does it make sense to keep returning these
shared_ptrs or should we again switch to a value model? It would be great
for consistency on the public API to go the value model way even if
initially the value simply holds a shared_ptr to the internal region impl.

Thoughts?


Re: [VOTE] Apache Geode release - v1.2.1 RC4

2017-09-18 Thread William Markito Oliveira
+1

Checked:
- Basic operations from command line
- Checksums match
- Release build green (blue actually)  (
https://builds.apache.org/job/Geode-release/lastBuild/)



On Mon, Sep 18, 2017 at 3:57 PM, Dan Smith  wrote:

> +1
>
> The jenkins release build was green and release check passed as well -
> https://github.com/upthewaterspout/geode-release-check
>
> -Dan
>
> On Sat, Sep 16, 2017 at 8:01 AM, Anthony Baker  wrote:
>
> > This is the fourth release candidate for Apache Geode, version 1.2.1.
> > Thanks to all the community members for their contributions to this
> > release!
> >
> > *** Please download, test and vote by Wednesday, September 20, 0800 hrs
> > US Pacific. ***
> >
> > It fixes the following issues:
> >  https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> > projectId=12318420=12341124
> >
> > Note that we are voting upon the source tags:  rel/v1.2.1.RC4
> >  https://github.com/apache/geode/tree/rel/v1.2.1.RC4
> >  https://github.com/apache/geode-examples/tree/rel/v1.2.1.RC4
> >
> > Commit ID:
> >  0b881b515eb1dcea974f0f5c1b40da03d42af9cf (geode)
> >  5d034de588a43cdefb8fbb3a6259579785768340 (geode-examples)
> >
> > Source and binary files:
> >  https://dist.apache.org/repos/dist/dev/geode/1.2.1.RC4
> >
> > Maven staging repo:
> >  https://repository.apache.org/content/repositories/orgapachegeode-1027
> >
> > Geode's KEYS file containing PGP keys we use to sign the release:
> >  https://github.com/apache/geode/blob/develop/KEYS
> >
> > pub  4096R/C72CFB64 2015-10-01
> >  Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64
> >
> >
> > Anthony
> >
>


Re: [VOTE] Apache Geode release - v1.2.1 RC4

2017-09-18 Thread Dan Smith
+1

The jenkins release build was green and release check passed as well -
https://github.com/upthewaterspout/geode-release-check

-Dan

On Sat, Sep 16, 2017 at 8:01 AM, Anthony Baker  wrote:

> This is the fourth release candidate for Apache Geode, version 1.2.1.
> Thanks to all the community members for their contributions to this
> release!
>
> *** Please download, test and vote by Wednesday, September 20, 0800 hrs
> US Pacific. ***
>
> It fixes the following issues:
>  https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> projectId=12318420=12341124
>
> Note that we are voting upon the source tags:  rel/v1.2.1.RC4
>  https://github.com/apache/geode/tree/rel/v1.2.1.RC4
>  https://github.com/apache/geode-examples/tree/rel/v1.2.1.RC4
>
> Commit ID:
>  0b881b515eb1dcea974f0f5c1b40da03d42af9cf (geode)
>  5d034de588a43cdefb8fbb3a6259579785768340 (geode-examples)
>
> Source and binary files:
>  https://dist.apache.org/repos/dist/dev/geode/1.2.1.RC4
>
> Maven staging repo:
>  https://repository.apache.org/content/repositories/orgapachegeode-1027
>
> Geode's KEYS file containing PGP keys we use to sign the release:
>  https://github.com/apache/geode/blob/develop/KEYS
>
> pub  4096R/C72CFB64 2015-10-01
>  Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64
>
>
> Anthony
>


Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread Jacob Barrett
Yup! It works.

https://gist.github.com/pivotal-jbarrett/c483f7f41b187f0ed8c80108aa6a

I also corrected the solution to use a unique_ptr for the Cache to
CacheImpl relationship.

Notice the main.cpp uses CacheFactory model as well as the direct
allocation model with CacheConfig. So if we ship with the current
CacheFactory model unchanged we can update later to support both where
CacheFactory will just create a CacheConfig and construct a new Cache the
same as you would directly.

Woot!

-Jake


On Mon, Sep 18, 2017 at 12:40 PM Jacob Barrett  wrote:

> So I am changing my vote to Value as well. It was great to put together
> all these samples because it really helped me understand what it buys us.
>
> With the current factory model it’s a little less appealing but not
> significantly enough to really notice. I would say we go GA with the
> current factory model and then quickly deprecate it for the non factory
> model. I think we can have both side by side until the next major release
> without issue. I’ll whip up a sample to make sure.
>
> -Jake
>
>
> > On Sep 18, 2017, at 12:24 PM, Ernest Burghardt 
> wrote:
> >
> > +1 to passing in CacheConfig  - might be a bit of refactoring (needed)
> but
> > complexity level should be low
> >
> > +1 value approach as well as dumping the Factory
> >
> >
> >
> >> On Mon, Sep 18, 2017 at 11:14 AM, David Kimura 
> wrote:
> >>
> >> +1 value approach (via some implementation from this thread)
> >>
> >> I think I like this.
> >>
> >> In all BAD cases, it's the user who shot themselves in the foot by using
> >> std::move unsafely.  I expect this behavior is the same behavior as for
> any
> >> other object.  And if we're ever able to get rid of the Cache/CacheImpl
> >> circular dependency then we can add a copy constructor.
> >>
> >> I also like the idea of passing in a CacheConfig.  My concern though is
> >> that it's piling on another big change.
> >>
> >> Thanks,
> >> David
> >>
> >> On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett 
> >> wrote:
> >>
> >>> Ok, one more idea.
> >>>
> https://gist.github.com/pivotal-jbarrett/beed5f70c1f3a238cef94832b13dab
> >> 7a
> >>>
> >>> The biggest issue with the value model is that we have been using a
> >> factory
> >>> to build the Cache object. We really don't need one and if we get rid
> of
> >> it
> >>> things look much better. They aren't perfect since we still need the
> back
> >>> pointer from the impl to the cache for later use. If we didn't need
> that
> >>> then we could allow copy construction. As it stands right now this
> >> version
> >>> allows value, shared_prt, unique_ptr, etc. without any real overhead or
> >> RVO
> >>> issues.
> >>>
> >>> The big change is that, rather than have a factory that we set a bunch
> of
> >>> values on and then ask it to create the Cache, we create a CacheConfig
> >>> object and pass that in to the Cache's constructor. Cache passes it to
> >>> CacheImpl and CacheImpl sets itself up based on the config. If you look
> >> at
> >>> what the current factory model does it isn't that different. For
> clarity
> >> I
> >>> added an XmlCacheConfig object to that builds up the CacheConfig via
> Xml.
> >>> You could imagine a YamlCacheConfig object *shiver*. The point is we
> >> don't
> >>> care as long as we get a CacheConfig with all the things we support at
> >>> "init" time.
> >>>
> >>> I know it is a more radical change but I feel it is more C++ and more
> >>> testable than the factory model. I also like that it solves some of the
> >>> issues with the value model we were looking at.
> >>>
> >>> -Jake
> >>>
> >>> On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett 
> >> wrote:
> >>>
>  Y'all here is an attempt to get the best of both worlds.
>  https://gist.github.com/pivotal-jbarrett/
> >> 52ba9ec5de0b494368d1c5282ef188
> >>> ef
> 
>  I thought I would try posting to Gist but so far not impressed, sorry.
> 
>  The Gist of it is that we can overcome the thirdpary or transient
>  reference back to the public Cache instance by keeping a reference to
> >> it
> >>> in
>  the implementation instance and updating it whenever the move
> >> constructor
>  is called.
> 
>  The downside is if your run this test it doesn't show RVO kicking in
> on
>  the second test where we move the value into a shared pointer. There
> >> are
> >>> a
>  couple of pitfalls you can stumble into as well by trying to used the
>  previous instance to access the cache after the move operation, as
>  illustrated by the "BAD" commented lines.
> 
>  The upside is the choices this gives the use for ownership of their
>  factory constructed Cache instance. They can keep it a value or move
> it
> >>> to
>  unique or shared pointer.
> 
>  Overhead wise I think we better off in value as long as there are no
>  moves, rare I would thing, but the moves 

Re: How gitbox works

2017-09-18 Thread Jacob Barrett
Mine are too. It's "normal".

On Mon, Sep 18, 2017 at 1:20 PM Kirk Lund  wrote:

> After 30 minutes, my MFA box went green (note: I had already enabled
> two-factor auth on github a long time ago) and all of the following
> appeared together. You would think that it might be desirable for the page
> to show "NOTE: Account linking happens every 30 minutes, be patient!" while
> you're waiting rather than AFTER you've waited. Also, I'm not sure why
> every geode repo is duplicated in the list (anyone know?).
>
> *NOTE: Account linking happens every 30 minutes, be patient!*
> *According to LDAP, you will have access to the following repositories:*
>
> *geode:*
> *geode*
> *geode*
> *geode-examples*
> *geode-examples*
> *geode-native*
> *geode-native*
> *geode-site*
> *geode-site*
> *incubator:*
> *No repositories for the incubator project served from gitbox yet...*
>
> On Mon, Sep 18, 2017 at 12:59 PM, Jared Stewart 
> wrote:
>
> > Https://github.com/apache is where I had to accept the invite and get
> the
> > third box green. I didn't seem to get an email invite either.
> >
> > - Jared
> >
> > On Sep 18, 2017 12:51 PM, "Kirk Lund"  wrote:
> >
> > If I navigate to https://github.com/apache, I see the following at the
> top
> > with a button to "View invitation" and accept:
> >
> > asfgit  invited you to join the The Apache
> > Software Foundation organization on Mar 7, 2016.
> >
> > Is this the expected invitation that I need to accept?! Apache never sent
> > me any email. I even tried removing and re-adding my github account to my
> > Apache Profile. This apache-gitbox integration really isn't ready for
> > prime-time usage is it.
> >
> > Thanks,
> > Kirk
> >
> >
> > On Mon, Sep 18, 2017 at 12:43 PM, Kirk Lund  wrote:
> >
> > > #3 is not GREEN, however I already have my github account listed on my
> > > Apache account:
> > >
> > > Your GitHub Username: kirklund
> > >
> > > So, what's missing or do I need to contact INFRA?
> > >
> > > Thanks,
> > > Kirk
> > >
> > > On Mon, Sep 18, 2017 at 12:38 PM, Mark Bretl 
> wrote:
> > >
> > >> Hi Kirk,
> > >>
> > >> I would go to the GitBox account linking utility page at
> > >> https://gitbox.apache.org/setup/ and verify all three steps are
> green.
> > If
> > >> they are green, then probably need to contact INFRA to figure out the
> > >> issue
> > >> with your account.
> > >>
> > >> --Mark
> > >>
> > >> On Mon, Sep 18, 2017 at 10:13 AM, Kirk Lund  wrote:
> > >>
> > >> > PS: If I had known the switchover to github would be this messy I
> > would
> > >> > have voted against it.
> > >> >
> > >> > On Mon, Sep 18, 2017 at 10:12 AM, Kirk Lund 
> wrote:
> > >> >
> > >> >> Yeah, I followed all of the directions shared by Jens and Bruce. I
> > also
> > >> >> had my github account added to my Apache account for the last 2-3
> > >> years.
> > >> >> Apparently the switchover to gitbox removed by commit permission.
> > >> >>
> > >> >> /Users/klund/dev/geode [525]$ git push
> > >> >> ERROR: Permission to apache/geode.git denied to kirklund.
> > >> >> fatal: Could not read from remote repository.
> > >> >>
> > >> >> Please make sure you have the correct access rights
> > >> >> and the repository exists.
> > >> >>
> > >> >> I also don't have the "Merge" button available to me when viewing
> > >> Apache
> > >> >> Geode pull requests.
> > >> >>
> > >> >> Anyone know who to contact to restore it?
> > >> >>
> > >> >> Thanks,
> > >> >> Kirk
> > >> >>
> > >> >> On Mon, Sep 18, 2017 at 10:05 AM, Kirk Lund 
> > wrote:
> > >> >>
> > >> >>> Apparently I no longer have commit access. Anyone know who/how I
> get
> > >> >>> access restored?
> > >> >>>
> > >> >>> Thanks,
> > >> >>> Kirk
> > >> >>>
> > >> >>> On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett <
> jbarr...@pivotal.io
> > >
> > >> >>> wrote:
> > >> >>>
> > >>  GitBox is just a fancy name Apache has given to their service
> that
> > >>  allows us to use GitHub as the source of authority for our Git
> > repo.
> > >>  Everything is done on GitHub using the GitHub processes.
> > >> 
> > >>  Anyone with write access to GitHub can merge a PR. The PR can be
> > >> merged
> > >>  from the PR GitHub website.
> > >> 
> > >>  -Jake
> > >> 
> > >> 
> > >>  > On Sep 18, 2017, at 9:43 AM, Kirk Lund 
> wrote:
> > >>  >
> > >>  > I've searched and not found a good summary on how gitbox works.
> > >>  Anyone know
> > >>  > of a good resource?
> > >>  >
> > >>  > The kinds of questions I have are: if other devs approve my PR,
> > do
> > >> I
> > >>  still
> > >>  > need to find another dev to commit for me or can I commit my
> own
> > PR
> > >>  if has
> > >>  > been approved?
> > >>  >
> > >>  > Thanks,
> > >>  > Kirk
> > >> 
> > >> >>>
> > >> 

Re: How gitbox works

2017-09-18 Thread Kirk Lund
After 30 minutes, my MFA box went green (note: I had already enabled
two-factor auth on github a long time ago) and all of the following
appeared together. You would think that it might be desirable for the page
to show "NOTE: Account linking happens every 30 minutes, be patient!" while
you're waiting rather than AFTER you've waited. Also, I'm not sure why
every geode repo is duplicated in the list (anyone know?).

*NOTE: Account linking happens every 30 minutes, be patient!*
*According to LDAP, you will have access to the following repositories:*

*geode:*
*geode*
*geode*
*geode-examples*
*geode-examples*
*geode-native*
*geode-native*
*geode-site*
*geode-site*
*incubator:*
*No repositories for the incubator project served from gitbox yet...*

On Mon, Sep 18, 2017 at 12:59 PM, Jared Stewart  wrote:

> Https://github.com/apache is where I had to accept the invite and get the
> third box green. I didn't seem to get an email invite either.
>
> - Jared
>
> On Sep 18, 2017 12:51 PM, "Kirk Lund"  wrote:
>
> If I navigate to https://github.com/apache, I see the following at the top
> with a button to "View invitation" and accept:
>
> asfgit  invited you to join the The Apache
> Software Foundation organization on Mar 7, 2016.
>
> Is this the expected invitation that I need to accept?! Apache never sent
> me any email. I even tried removing and re-adding my github account to my
> Apache Profile. This apache-gitbox integration really isn't ready for
> prime-time usage is it.
>
> Thanks,
> Kirk
>
>
> On Mon, Sep 18, 2017 at 12:43 PM, Kirk Lund  wrote:
>
> > #3 is not GREEN, however I already have my github account listed on my
> > Apache account:
> >
> > Your GitHub Username: kirklund
> >
> > So, what's missing or do I need to contact INFRA?
> >
> > Thanks,
> > Kirk
> >
> > On Mon, Sep 18, 2017 at 12:38 PM, Mark Bretl  wrote:
> >
> >> Hi Kirk,
> >>
> >> I would go to the GitBox account linking utility page at
> >> https://gitbox.apache.org/setup/ and verify all three steps are green.
> If
> >> they are green, then probably need to contact INFRA to figure out the
> >> issue
> >> with your account.
> >>
> >> --Mark
> >>
> >> On Mon, Sep 18, 2017 at 10:13 AM, Kirk Lund  wrote:
> >>
> >> > PS: If I had known the switchover to github would be this messy I
> would
> >> > have voted against it.
> >> >
> >> > On Mon, Sep 18, 2017 at 10:12 AM, Kirk Lund  wrote:
> >> >
> >> >> Yeah, I followed all of the directions shared by Jens and Bruce. I
> also
> >> >> had my github account added to my Apache account for the last 2-3
> >> years.
> >> >> Apparently the switchover to gitbox removed by commit permission.
> >> >>
> >> >> /Users/klund/dev/geode [525]$ git push
> >> >> ERROR: Permission to apache/geode.git denied to kirklund.
> >> >> fatal: Could not read from remote repository.
> >> >>
> >> >> Please make sure you have the correct access rights
> >> >> and the repository exists.
> >> >>
> >> >> I also don't have the "Merge" button available to me when viewing
> >> Apache
> >> >> Geode pull requests.
> >> >>
> >> >> Anyone know who to contact to restore it?
> >> >>
> >> >> Thanks,
> >> >> Kirk
> >> >>
> >> >> On Mon, Sep 18, 2017 at 10:05 AM, Kirk Lund 
> wrote:
> >> >>
> >> >>> Apparently I no longer have commit access. Anyone know who/how I get
> >> >>> access restored?
> >> >>>
> >> >>> Thanks,
> >> >>> Kirk
> >> >>>
> >> >>> On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett  >
> >> >>> wrote:
> >> >>>
> >>  GitBox is just a fancy name Apache has given to their service that
> >>  allows us to use GitHub as the source of authority for our Git
> repo.
> >>  Everything is done on GitHub using the GitHub processes.
> >> 
> >>  Anyone with write access to GitHub can merge a PR. The PR can be
> >> merged
> >>  from the PR GitHub website.
> >> 
> >>  -Jake
> >> 
> >> 
> >>  > On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
> >>  >
> >>  > I've searched and not found a good summary on how gitbox works.
> >>  Anyone know
> >>  > of a good resource?
> >>  >
> >>  > The kinds of questions I have are: if other devs approve my PR,
> do
> >> I
> >>  still
> >>  > need to find another dev to commit for me or can I commit my own
> PR
> >>  if has
> >>  > been approved?
> >>  >
> >>  > Thanks,
> >>  > Kirk
> >> 
> >> >>>
> >> >>>
> >> >>
> >> >
> >>
> >
> >
>


Re: How gitbox works

2017-09-18 Thread Jared Stewart
Https://github.com/apache is where I had to accept the invite and get the
third box green. I didn't seem to get an email invite either.

- Jared

On Sep 18, 2017 12:51 PM, "Kirk Lund"  wrote:

If I navigate to https://github.com/apache, I see the following at the top
with a button to "View invitation" and accept:

asfgit  invited you to join the The Apache
Software Foundation organization on Mar 7, 2016.

Is this the expected invitation that I need to accept?! Apache never sent
me any email. I even tried removing and re-adding my github account to my
Apache Profile. This apache-gitbox integration really isn't ready for
prime-time usage is it.

Thanks,
Kirk


On Mon, Sep 18, 2017 at 12:43 PM, Kirk Lund  wrote:

> #3 is not GREEN, however I already have my github account listed on my
> Apache account:
>
> Your GitHub Username: kirklund
>
> So, what's missing or do I need to contact INFRA?
>
> Thanks,
> Kirk
>
> On Mon, Sep 18, 2017 at 12:38 PM, Mark Bretl  wrote:
>
>> Hi Kirk,
>>
>> I would go to the GitBox account linking utility page at
>> https://gitbox.apache.org/setup/ and verify all three steps are green. If
>> they are green, then probably need to contact INFRA to figure out the
>> issue
>> with your account.
>>
>> --Mark
>>
>> On Mon, Sep 18, 2017 at 10:13 AM, Kirk Lund  wrote:
>>
>> > PS: If I had known the switchover to github would be this messy I would
>> > have voted against it.
>> >
>> > On Mon, Sep 18, 2017 at 10:12 AM, Kirk Lund  wrote:
>> >
>> >> Yeah, I followed all of the directions shared by Jens and Bruce. I
also
>> >> had my github account added to my Apache account for the last 2-3
>> years.
>> >> Apparently the switchover to gitbox removed by commit permission.
>> >>
>> >> /Users/klund/dev/geode [525]$ git push
>> >> ERROR: Permission to apache/geode.git denied to kirklund.
>> >> fatal: Could not read from remote repository.
>> >>
>> >> Please make sure you have the correct access rights
>> >> and the repository exists.
>> >>
>> >> I also don't have the "Merge" button available to me when viewing
>> Apache
>> >> Geode pull requests.
>> >>
>> >> Anyone know who to contact to restore it?
>> >>
>> >> Thanks,
>> >> Kirk
>> >>
>> >> On Mon, Sep 18, 2017 at 10:05 AM, Kirk Lund  wrote:
>> >>
>> >>> Apparently I no longer have commit access. Anyone know who/how I get
>> >>> access restored?
>> >>>
>> >>> Thanks,
>> >>> Kirk
>> >>>
>> >>> On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett 
>> >>> wrote:
>> >>>
>>  GitBox is just a fancy name Apache has given to their service that
>>  allows us to use GitHub as the source of authority for our Git repo.
>>  Everything is done on GitHub using the GitHub processes.
>> 
>>  Anyone with write access to GitHub can merge a PR. The PR can be
>> merged
>>  from the PR GitHub website.
>> 
>>  -Jake
>> 
>> 
>>  > On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
>>  >
>>  > I've searched and not found a good summary on how gitbox works.
>>  Anyone know
>>  > of a good resource?
>>  >
>>  > The kinds of questions I have are: if other devs approve my PR, do
>> I
>>  still
>>  > need to find another dev to commit for me or can I commit my own
PR
>>  if has
>>  > been approved?
>>  >
>>  > Thanks,
>>  > Kirk
>> 
>> >>>
>> >>>
>> >>
>> >
>>
>
>


Re: How gitbox works

2017-09-18 Thread Kirk Lund
Third box on https://gitbox.apache.org/setup/ still isn't GREEN.

On Mon, Sep 18, 2017 at 12:51 PM, Kirk Lund  wrote:

> If I navigate to https://github.com/apache, I see the following at the
> top with a button to "View invitation" and accept:
>
> asfgit  invited you to join the The Apache
> Software Foundation organization on Mar 7, 2016.
>
> Is this the expected invitation that I need to accept?! Apache never sent
> me any email. I even tried removing and re-adding my github account to my
> Apache Profile. This apache-gitbox integration really isn't ready for
> prime-time usage is it.
>
> Thanks,
> Kirk
>
>
> On Mon, Sep 18, 2017 at 12:43 PM, Kirk Lund  wrote:
>
>> #3 is not GREEN, however I already have my github account listed on my
>> Apache account:
>>
>> Your GitHub Username: kirklund
>>
>> So, what's missing or do I need to contact INFRA?
>>
>> Thanks,
>> Kirk
>>
>> On Mon, Sep 18, 2017 at 12:38 PM, Mark Bretl  wrote:
>>
>>> Hi Kirk,
>>>
>>> I would go to the GitBox account linking utility page at
>>> https://gitbox.apache.org/setup/ and verify all three steps are green.
>>> If
>>> they are green, then probably need to contact INFRA to figure out the
>>> issue
>>> with your account.
>>>
>>> --Mark
>>>
>>> On Mon, Sep 18, 2017 at 10:13 AM, Kirk Lund  wrote:
>>>
>>> > PS: If I had known the switchover to github would be this messy I would
>>> > have voted against it.
>>> >
>>> > On Mon, Sep 18, 2017 at 10:12 AM, Kirk Lund  wrote:
>>> >
>>> >> Yeah, I followed all of the directions shared by Jens and Bruce. I
>>> also
>>> >> had my github account added to my Apache account for the last 2-3
>>> years.
>>> >> Apparently the switchover to gitbox removed by commit permission.
>>> >>
>>> >> /Users/klund/dev/geode [525]$ git push
>>> >> ERROR: Permission to apache/geode.git denied to kirklund.
>>> >> fatal: Could not read from remote repository.
>>> >>
>>> >> Please make sure you have the correct access rights
>>> >> and the repository exists.
>>> >>
>>> >> I also don't have the "Merge" button available to me when viewing
>>> Apache
>>> >> Geode pull requests.
>>> >>
>>> >> Anyone know who to contact to restore it?
>>> >>
>>> >> Thanks,
>>> >> Kirk
>>> >>
>>> >> On Mon, Sep 18, 2017 at 10:05 AM, Kirk Lund  wrote:
>>> >>
>>> >>> Apparently I no longer have commit access. Anyone know who/how I get
>>> >>> access restored?
>>> >>>
>>> >>> Thanks,
>>> >>> Kirk
>>> >>>
>>> >>> On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett 
>>> >>> wrote:
>>> >>>
>>>  GitBox is just a fancy name Apache has given to their service that
>>>  allows us to use GitHub as the source of authority for our Git repo.
>>>  Everything is done on GitHub using the GitHub processes.
>>> 
>>>  Anyone with write access to GitHub can merge a PR. The PR can be
>>> merged
>>>  from the PR GitHub website.
>>> 
>>>  -Jake
>>> 
>>> 
>>>  > On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
>>>  >
>>>  > I've searched and not found a good summary on how gitbox works.
>>>  Anyone know
>>>  > of a good resource?
>>>  >
>>>  > The kinds of questions I have are: if other devs approve my PR,
>>> do I
>>>  still
>>>  > need to find another dev to commit for me or can I commit my own
>>> PR
>>>  if has
>>>  > been approved?
>>>  >
>>>  > Thanks,
>>>  > Kirk
>>> 
>>> >>>
>>> >>>
>>> >>
>>> >
>>>
>>
>>
>


Re: How gitbox works

2017-09-18 Thread Kirk Lund
If I navigate to https://github.com/apache, I see the following at the top
with a button to "View invitation" and accept:

asfgit  invited you to join the The Apache
Software Foundation organization on Mar 7, 2016.

Is this the expected invitation that I need to accept?! Apache never sent
me any email. I even tried removing and re-adding my github account to my
Apache Profile. This apache-gitbox integration really isn't ready for
prime-time usage is it.

Thanks,
Kirk


On Mon, Sep 18, 2017 at 12:43 PM, Kirk Lund  wrote:

> #3 is not GREEN, however I already have my github account listed on my
> Apache account:
>
> Your GitHub Username: kirklund
>
> So, what's missing or do I need to contact INFRA?
>
> Thanks,
> Kirk
>
> On Mon, Sep 18, 2017 at 12:38 PM, Mark Bretl  wrote:
>
>> Hi Kirk,
>>
>> I would go to the GitBox account linking utility page at
>> https://gitbox.apache.org/setup/ and verify all three steps are green. If
>> they are green, then probably need to contact INFRA to figure out the
>> issue
>> with your account.
>>
>> --Mark
>>
>> On Mon, Sep 18, 2017 at 10:13 AM, Kirk Lund  wrote:
>>
>> > PS: If I had known the switchover to github would be this messy I would
>> > have voted against it.
>> >
>> > On Mon, Sep 18, 2017 at 10:12 AM, Kirk Lund  wrote:
>> >
>> >> Yeah, I followed all of the directions shared by Jens and Bruce. I also
>> >> had my github account added to my Apache account for the last 2-3
>> years.
>> >> Apparently the switchover to gitbox removed by commit permission.
>> >>
>> >> /Users/klund/dev/geode [525]$ git push
>> >> ERROR: Permission to apache/geode.git denied to kirklund.
>> >> fatal: Could not read from remote repository.
>> >>
>> >> Please make sure you have the correct access rights
>> >> and the repository exists.
>> >>
>> >> I also don't have the "Merge" button available to me when viewing
>> Apache
>> >> Geode pull requests.
>> >>
>> >> Anyone know who to contact to restore it?
>> >>
>> >> Thanks,
>> >> Kirk
>> >>
>> >> On Mon, Sep 18, 2017 at 10:05 AM, Kirk Lund  wrote:
>> >>
>> >>> Apparently I no longer have commit access. Anyone know who/how I get
>> >>> access restored?
>> >>>
>> >>> Thanks,
>> >>> Kirk
>> >>>
>> >>> On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett 
>> >>> wrote:
>> >>>
>>  GitBox is just a fancy name Apache has given to their service that
>>  allows us to use GitHub as the source of authority for our Git repo.
>>  Everything is done on GitHub using the GitHub processes.
>> 
>>  Anyone with write access to GitHub can merge a PR. The PR can be
>> merged
>>  from the PR GitHub website.
>> 
>>  -Jake
>> 
>> 
>>  > On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
>>  >
>>  > I've searched and not found a good summary on how gitbox works.
>>  Anyone know
>>  > of a good resource?
>>  >
>>  > The kinds of questions I have are: if other devs approve my PR, do
>> I
>>  still
>>  > need to find another dev to commit for me or can I commit my own PR
>>  if has
>>  > been approved?
>>  >
>>  > Thanks,
>>  > Kirk
>> 
>> >>>
>> >>>
>> >>
>> >
>>
>
>


Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread Jacob Barrett
So I am changing my vote to Value as well. It was great to put together all 
these samples because it really helped me understand what it buys us. 

With the current factory model it’s a little less appealing but not 
significantly enough to really notice. I would say we go GA with the current 
factory model and then quickly deprecate it for the non factory model. I think 
we can have both side by side until the next major release without issue. I’ll 
whip up a sample to make sure.

-Jake


> On Sep 18, 2017, at 12:24 PM, Ernest Burghardt  wrote:
> 
> +1 to passing in CacheConfig  - might be a bit of refactoring (needed) but
> complexity level should be low
> 
> +1 value approach as well as dumping the Factory
> 
> 
> 
>> On Mon, Sep 18, 2017 at 11:14 AM, David Kimura  wrote:
>> 
>> +1 value approach (via some implementation from this thread)
>> 
>> I think I like this.
>> 
>> In all BAD cases, it's the user who shot themselves in the foot by using
>> std::move unsafely.  I expect this behavior is the same behavior as for any
>> other object.  And if we're ever able to get rid of the Cache/CacheImpl
>> circular dependency then we can add a copy constructor.
>> 
>> I also like the idea of passing in a CacheConfig.  My concern though is
>> that it's piling on another big change.
>> 
>> Thanks,
>> David
>> 
>> On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett 
>> wrote:
>> 
>>> Ok, one more idea.
>>> https://gist.github.com/pivotal-jbarrett/beed5f70c1f3a238cef94832b13dab
>> 7a
>>> 
>>> The biggest issue with the value model is that we have been using a
>> factory
>>> to build the Cache object. We really don't need one and if we get rid of
>> it
>>> things look much better. They aren't perfect since we still need the back
>>> pointer from the impl to the cache for later use. If we didn't need that
>>> then we could allow copy construction. As it stands right now this
>> version
>>> allows value, shared_prt, unique_ptr, etc. without any real overhead or
>> RVO
>>> issues.
>>> 
>>> The big change is that, rather than have a factory that we set a bunch of
>>> values on and then ask it to create the Cache, we create a CacheConfig
>>> object and pass that in to the Cache's constructor. Cache passes it to
>>> CacheImpl and CacheImpl sets itself up based on the config. If you look
>> at
>>> what the current factory model does it isn't that different. For clarity
>> I
>>> added an XmlCacheConfig object to that builds up the CacheConfig via Xml.
>>> You could imagine a YamlCacheConfig object *shiver*. The point is we
>> don't
>>> care as long as we get a CacheConfig with all the things we support at
>>> "init" time.
>>> 
>>> I know it is a more radical change but I feel it is more C++ and more
>>> testable than the factory model. I also like that it solves some of the
>>> issues with the value model we were looking at.
>>> 
>>> -Jake
>>> 
>>> On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett 
>> wrote:
>>> 
 Y'all here is an attempt to get the best of both worlds.
 https://gist.github.com/pivotal-jbarrett/
>> 52ba9ec5de0b494368d1c5282ef188
>>> ef
 
 I thought I would try posting to Gist but so far not impressed, sorry.
 
 The Gist of it is that we can overcome the thirdpary or transient
 reference back to the public Cache instance by keeping a reference to
>> it
>>> in
 the implementation instance and updating it whenever the move
>> constructor
 is called.
 
 The downside is if your run this test it doesn't show RVO kicking in on
 the second test where we move the value into a shared pointer. There
>> are
>>> a
 couple of pitfalls you can stumble into as well by trying to used the
 previous instance to access the cache after the move operation, as
 illustrated by the "BAD" commented lines.
 
 The upside is the choices this gives the use for ownership of their
 factory constructed Cache instance. They can keep it a value or move it
>>> to
 unique or shared pointer.
 
 Overhead wise I think we better off in value as long as there are no
 moves, rare I would thing, but the moves are pretty cheap at the point
 since we only maintain a unique_ptr. After moving into a shared_ptr it
>>> acts
 identical to the shared_ptr model proposed earlier.
 
 -Jake
 
 
 On Thu, Sep 14, 2017 at 3:36 PM Michael Martell 
 wrote:
 
> Late to this party.
> 
> Confession 1: I had to look up both RVO and copy-elision.
> Confession 2: I don't like using pointers at all. I used to, but I've
> evolved to just use C# and Java :)
> 
> Without investing a lot more time, I don't have strong feelings about
>>> raw
> vs shared pointers. My only question is: Can we return ptr to abstract
> class everywhere we return objects? Just thinking of mocking, which
>>> always
> wants to mock interfaces.

Re: How gitbox works

2017-09-18 Thread Mark Bretl
Hi Kirk,

I would go to the GitBox account linking utility page at
https://gitbox.apache.org/setup/ and verify all three steps are green. If
they are green, then probably need to contact INFRA to figure out the issue
with your account.

--Mark

On Mon, Sep 18, 2017 at 10:13 AM, Kirk Lund  wrote:

> PS: If I had known the switchover to github would be this messy I would
> have voted against it.
>
> On Mon, Sep 18, 2017 at 10:12 AM, Kirk Lund  wrote:
>
>> Yeah, I followed all of the directions shared by Jens and Bruce. I also
>> had my github account added to my Apache account for the last 2-3 years.
>> Apparently the switchover to gitbox removed by commit permission.
>>
>> /Users/klund/dev/geode [525]$ git push
>> ERROR: Permission to apache/geode.git denied to kirklund.
>> fatal: Could not read from remote repository.
>>
>> Please make sure you have the correct access rights
>> and the repository exists.
>>
>> I also don't have the "Merge" button available to me when viewing Apache
>> Geode pull requests.
>>
>> Anyone know who to contact to restore it?
>>
>> Thanks,
>> Kirk
>>
>> On Mon, Sep 18, 2017 at 10:05 AM, Kirk Lund  wrote:
>>
>>> Apparently I no longer have commit access. Anyone know who/how I get
>>> access restored?
>>>
>>> Thanks,
>>> Kirk
>>>
>>> On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett 
>>> wrote:
>>>
 GitBox is just a fancy name Apache has given to their service that
 allows us to use GitHub as the source of authority for our Git repo.
 Everything is done on GitHub using the GitHub processes.

 Anyone with write access to GitHub can merge a PR. The PR can be merged
 from the PR GitHub website.

 -Jake


 > On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
 >
 > I've searched and not found a good summary on how gitbox works.
 Anyone know
 > of a good resource?
 >
 > The kinds of questions I have are: if other devs approve my PR, do I
 still
 > need to find another dev to commit for me or can I commit my own PR
 if has
 > been approved?
 >
 > Thanks,
 > Kirk

>>>
>>>
>>
>


Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread Ernest Burghardt
+1 to passing in CacheConfig  - might be a bit of refactoring (needed) but
complexity level should be low

+1 value approach as well as dumping the Factory



On Mon, Sep 18, 2017 at 11:14 AM, David Kimura  wrote:

> +1 value approach (via some implementation from this thread)
>
> I think I like this.
>
> In all BAD cases, it's the user who shot themselves in the foot by using
> std::move unsafely.  I expect this behavior is the same behavior as for any
> other object.  And if we're ever able to get rid of the Cache/CacheImpl
> circular dependency then we can add a copy constructor.
>
> I also like the idea of passing in a CacheConfig.  My concern though is
> that it's piling on another big change.
>
> Thanks,
> David
>
> On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett 
> wrote:
>
> > Ok, one more idea.
> > https://gist.github.com/pivotal-jbarrett/beed5f70c1f3a238cef94832b13dab
> 7a
> >
> > The biggest issue with the value model is that we have been using a
> factory
> > to build the Cache object. We really don't need one and if we get rid of
> it
> > things look much better. They aren't perfect since we still need the back
> > pointer from the impl to the cache for later use. If we didn't need that
> > then we could allow copy construction. As it stands right now this
> version
> > allows value, shared_prt, unique_ptr, etc. without any real overhead or
> RVO
> > issues.
> >
> > The big change is that, rather than have a factory that we set a bunch of
> > values on and then ask it to create the Cache, we create a CacheConfig
> > object and pass that in to the Cache's constructor. Cache passes it to
> > CacheImpl and CacheImpl sets itself up based on the config. If you look
> at
> > what the current factory model does it isn't that different. For clarity
> I
> > added an XmlCacheConfig object to that builds up the CacheConfig via Xml.
> > You could imagine a YamlCacheConfig object *shiver*. The point is we
> don't
> > care as long as we get a CacheConfig with all the things we support at
> > "init" time.
> >
> > I know it is a more radical change but I feel it is more C++ and more
> > testable than the factory model. I also like that it solves some of the
> > issues with the value model we were looking at.
> >
> > -Jake
> >
> > On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett 
> wrote:
> >
> > > Y'all here is an attempt to get the best of both worlds.
> > > https://gist.github.com/pivotal-jbarrett/
> 52ba9ec5de0b494368d1c5282ef188
> > ef
> > >
> > > I thought I would try posting to Gist but so far not impressed, sorry.
> > >
> > > The Gist of it is that we can overcome the thirdpary or transient
> > > reference back to the public Cache instance by keeping a reference to
> it
> > in
> > > the implementation instance and updating it whenever the move
> constructor
> > > is called.
> > >
> > > The downside is if your run this test it doesn't show RVO kicking in on
> > > the second test where we move the value into a shared pointer. There
> are
> > a
> > > couple of pitfalls you can stumble into as well by trying to used the
> > > previous instance to access the cache after the move operation, as
> > > illustrated by the "BAD" commented lines.
> > >
> > > The upside is the choices this gives the use for ownership of their
> > > factory constructed Cache instance. They can keep it a value or move it
> > to
> > > unique or shared pointer.
> > >
> > > Overhead wise I think we better off in value as long as there are no
> > > moves, rare I would thing, but the moves are pretty cheap at the point
> > > since we only maintain a unique_ptr. After moving into a shared_ptr it
> > acts
> > > identical to the shared_ptr model proposed earlier.
> > >
> > > -Jake
> > >
> > >
> > > On Thu, Sep 14, 2017 at 3:36 PM Michael Martell 
> > > wrote:
> > >
> > >> Late to this party.
> > >>
> > >> Confession 1: I had to look up both RVO and copy-elision.
> > >> Confession 2: I don't like using pointers at all. I used to, but I've
> > >> evolved to just use C# and Java :)
> > >>
> > >> Without investing a lot more time, I don't have strong feelings about
> > raw
> > >> vs shared pointers. My only question is: Can we return ptr to abstract
> > >> class everywhere we return objects? Just thinking of mocking, which
> > always
> > >> wants to mock interfaces.
> > >>
> > >> On Thu, Sep 14, 2017 at 2:25 PM, Michael William Dodge <
> > mdo...@pivotal.io
> > >> >
> > >> wrote:
> > >>
> > >> > +0 shared pointer
> > >> >
> > >> > > On 14 Sep, 2017, at 14:09, Ernest Burghardt <
> eburgha...@pivotal.io>
> > >> > wrote:
> > >> > >
> > >> > > Calling a vote for:
> > >> > >
> > >> > > - Raw pointer
> > >> > > - shard pointer
> > >> > >
> > >> > > +1 raw Pointer, I had to look up RVO and am new to std::move(s)
> > >> > >
> > >> > > On Thu, Sep 14, 2017 at 3:02 PM, Michael William Dodge <
> > >> > mdo...@pivotal.io>
> > >> > > wrote:
> > >> > >
> > >> > >> I generally dig 

Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread Mark Hanson
Sounds good to me. +1

> On Sep 18, 2017, at 11:36 AM, David Kimura  wrote:
> 
> Application code doesn't need to invoke move (and probably shouldn't).  In
> the examples, I think it's more of an exercise to show what would happen if
> invoked.  And really, I expect same badness to happen using shared pointer
> method if we dereferenced a moved pointer.
> 
> Thanks,
> David
> 
> On Mon, Sep 18, 2017 at 11:19 AM, Mark Hanson  wrote:
> 
>> If you think this is a significant ease of use benefit, why have to invoke
>> move? I understand this code, but I have not seen it in recent memory.
>> I agree that this may make use of a copy(move) constructor easier, but...
>> 
>> If the consensus is that this is a significant ease of use benefit for the
>> user, I am on board…
>> 
>> +1
>> 
>> Thanks,
>> Mark
>>> On Sep 18, 2017, at 11:15 AM, David Kimura  wrote:
>>> 
>>> The benefit I see isn't for performance, it's flexibility and ease of use
>>> by application developer.  Anything we can do to achieve this, I think
>> is a
>>> significant win.
>>> 
>>> I think the reason for the various approaches is to determine whether we
>>> can safely create the simple/flexible API without incurring a penalty on
>>> performance.  Personally, I think the no nullptr benefit that Sarge
>>> mentioned in previous thread is enough to warrant this approach serious
>>> thought even though it provides no performance benefit.
>>> 
>>> Thanks,
>>> David
>>> 
>>> On Mon, Sep 18, 2017 at 10:47 AM, Mark Hanson 
>> wrote:
>>> 
 Hi All,
 
 As we are creating a user API, unless there is a significant performance
 benefit, I think we should try to take the simpler route.
 
 I see benefit to the approach being proposed, but I don’t see a
 significant benefit. Someone please school me if I am wrong.
 
 I am still in the shared pointer camp for cache and a raw pointer to
>> cache
 in cacheImpl.
 
 Sorry, and thanks,
 Mark
 
 
 
> On Sep 18, 2017, at 10:14 AM, David Kimura  wrote:
> 
> +1 value approach (via some implementation from this thread)
> 
> I think I like this.
> 
> In all BAD cases, it's the user who shot themselves in the foot by
>> using
> std::move unsafely.  I expect this behavior is the same behavior as for
 any
> other object.  And if we're ever able to get rid of the Cache/CacheImpl
> circular dependency then we can add a copy constructor.
> 
> I also like the idea of passing in a CacheConfig.  My concern though is
> that it's piling on another big change.
> 
> Thanks,
> David
> 
> On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett 
 wrote:
> 
>> Ok, one more idea.
>> https://gist.github.com/pivotal-jbarrett/
>> beed5f70c1f3a238cef94832b13dab
 7a
>> 
>> The biggest issue with the value model is that we have been using a
 factory
>> to build the Cache object. We really don't need one and if we get rid
 of it
>> things look much better. They aren't perfect since we still need the
 back
>> pointer from the impl to the cache for later use. If we didn't need
>> that
>> then we could allow copy construction. As it stands right now this
 version
>> allows value, shared_prt, unique_ptr, etc. without any real overhead
>> or
 RVO
>> issues.
>> 
>> The big change is that, rather than have a factory that we set a bunch
 of
>> values on and then ask it to create the Cache, we create a CacheConfig
>> object and pass that in to the Cache's constructor. Cache passes it to
>> CacheImpl and CacheImpl sets itself up based on the config. If you
>> look
 at
>> what the current factory model does it isn't that different. For
 clarity I
>> added an XmlCacheConfig object to that builds up the CacheConfig via
 Xml.
>> You could imagine a YamlCacheConfig object *shiver*. The point is we
 don't
>> care as long as we get a CacheConfig with all the things we support at
>> "init" time.
>> 
>> I know it is a more radical change but I feel it is more C++ and more
>> testable than the factory model. I also like that it solves some of
>> the
>> issues with the value model we were looking at.
>> 
>> -Jake
>> 
>> On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett 
 wrote:
>> 
>>> Y'all here is an attempt to get the best of both worlds.
>>> https://gist.github.com/pivotal-jbarrett/
 52ba9ec5de0b494368d1c5282ef188
>> ef
>>> 
>>> I thought I would try posting to Gist but so far not impressed,
>> sorry.
>>> 
>>> The Gist of it is that we can overcome the thirdpary or transient
>>> reference back to the public Cache instance by keeping a reference to
 it
>> in
>>> the implementation 

Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread David Kimura
Application code doesn't need to invoke move (and probably shouldn't).  In
the examples, I think it's more of an exercise to show what would happen if
invoked.  And really, I expect same badness to happen using shared pointer
method if we dereferenced a moved pointer.

Thanks,
David

On Mon, Sep 18, 2017 at 11:19 AM, Mark Hanson  wrote:

> If you think this is a significant ease of use benefit, why have to invoke
> move? I understand this code, but I have not seen it in recent memory.
> I agree that this may make use of a copy(move) constructor easier, but...
>
> If the consensus is that this is a significant ease of use benefit for the
> user, I am on board…
>
> +1
>
> Thanks,
> Mark
> > On Sep 18, 2017, at 11:15 AM, David Kimura  wrote:
> >
> > The benefit I see isn't for performance, it's flexibility and ease of use
> > by application developer.  Anything we can do to achieve this, I think
> is a
> > significant win.
> >
> > I think the reason for the various approaches is to determine whether we
> > can safely create the simple/flexible API without incurring a penalty on
> > performance.  Personally, I think the no nullptr benefit that Sarge
> > mentioned in previous thread is enough to warrant this approach serious
> > thought even though it provides no performance benefit.
> >
> > Thanks,
> > David
> >
> > On Mon, Sep 18, 2017 at 10:47 AM, Mark Hanson 
> wrote:
> >
> >> Hi All,
> >>
> >> As we are creating a user API, unless there is a significant performance
> >> benefit, I think we should try to take the simpler route.
> >>
> >> I see benefit to the approach being proposed, but I don’t see a
> >> significant benefit. Someone please school me if I am wrong.
> >>
> >> I am still in the shared pointer camp for cache and a raw pointer to
> cache
> >> in cacheImpl.
> >>
> >> Sorry, and thanks,
> >> Mark
> >>
> >>
> >>
> >>> On Sep 18, 2017, at 10:14 AM, David Kimura  wrote:
> >>>
> >>> +1 value approach (via some implementation from this thread)
> >>>
> >>> I think I like this.
> >>>
> >>> In all BAD cases, it's the user who shot themselves in the foot by
> using
> >>> std::move unsafely.  I expect this behavior is the same behavior as for
> >> any
> >>> other object.  And if we're ever able to get rid of the Cache/CacheImpl
> >>> circular dependency then we can add a copy constructor.
> >>>
> >>> I also like the idea of passing in a CacheConfig.  My concern though is
> >>> that it's piling on another big change.
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>> On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett 
> >> wrote:
> >>>
>  Ok, one more idea.
>  https://gist.github.com/pivotal-jbarrett/
> beed5f70c1f3a238cef94832b13dab
> >> 7a
> 
>  The biggest issue with the value model is that we have been using a
> >> factory
>  to build the Cache object. We really don't need one and if we get rid
> >> of it
>  things look much better. They aren't perfect since we still need the
> >> back
>  pointer from the impl to the cache for later use. If we didn't need
> that
>  then we could allow copy construction. As it stands right now this
> >> version
>  allows value, shared_prt, unique_ptr, etc. without any real overhead
> or
> >> RVO
>  issues.
> 
>  The big change is that, rather than have a factory that we set a bunch
> >> of
>  values on and then ask it to create the Cache, we create a CacheConfig
>  object and pass that in to the Cache's constructor. Cache passes it to
>  CacheImpl and CacheImpl sets itself up based on the config. If you
> look
> >> at
>  what the current factory model does it isn't that different. For
> >> clarity I
>  added an XmlCacheConfig object to that builds up the CacheConfig via
> >> Xml.
>  You could imagine a YamlCacheConfig object *shiver*. The point is we
> >> don't
>  care as long as we get a CacheConfig with all the things we support at
>  "init" time.
> 
>  I know it is a more radical change but I feel it is more C++ and more
>  testable than the factory model. I also like that it solves some of
> the
>  issues with the value model we were looking at.
> 
>  -Jake
> 
>  On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett 
> >> wrote:
> 
> > Y'all here is an attempt to get the best of both worlds.
> > https://gist.github.com/pivotal-jbarrett/
> >> 52ba9ec5de0b494368d1c5282ef188
>  ef
> >
> > I thought I would try posting to Gist but so far not impressed,
> sorry.
> >
> > The Gist of it is that we can overcome the thirdpary or transient
> > reference back to the public Cache instance by keeping a reference to
> >> it
>  in
> > the implementation instance and updating it whenever the move
> >> constructor
> > is called.
> >
> > The downside is if your run this test it doesn't show RVO kicking in
> on
> 

Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread Mark Hanson
If you think this is a significant ease of use benefit, why have to invoke 
move? I understand this code, but I have not seen it in recent memory.
I agree that this may make use of a copy(move) constructor easier, but...

If the consensus is that this is a significant ease of use benefit for the 
user, I am on board…

+1

Thanks,
Mark
> On Sep 18, 2017, at 11:15 AM, David Kimura  wrote:
> 
> The benefit I see isn't for performance, it's flexibility and ease of use
> by application developer.  Anything we can do to achieve this, I think is a
> significant win.
> 
> I think the reason for the various approaches is to determine whether we
> can safely create the simple/flexible API without incurring a penalty on
> performance.  Personally, I think the no nullptr benefit that Sarge
> mentioned in previous thread is enough to warrant this approach serious
> thought even though it provides no performance benefit.
> 
> Thanks,
> David
> 
> On Mon, Sep 18, 2017 at 10:47 AM, Mark Hanson  wrote:
> 
>> Hi All,
>> 
>> As we are creating a user API, unless there is a significant performance
>> benefit, I think we should try to take the simpler route.
>> 
>> I see benefit to the approach being proposed, but I don’t see a
>> significant benefit. Someone please school me if I am wrong.
>> 
>> I am still in the shared pointer camp for cache and a raw pointer to cache
>> in cacheImpl.
>> 
>> Sorry, and thanks,
>> Mark
>> 
>> 
>> 
>>> On Sep 18, 2017, at 10:14 AM, David Kimura  wrote:
>>> 
>>> +1 value approach (via some implementation from this thread)
>>> 
>>> I think I like this.
>>> 
>>> In all BAD cases, it's the user who shot themselves in the foot by using
>>> std::move unsafely.  I expect this behavior is the same behavior as for
>> any
>>> other object.  And if we're ever able to get rid of the Cache/CacheImpl
>>> circular dependency then we can add a copy constructor.
>>> 
>>> I also like the idea of passing in a CacheConfig.  My concern though is
>>> that it's piling on another big change.
>>> 
>>> Thanks,
>>> David
>>> 
>>> On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett 
>> wrote:
>>> 
 Ok, one more idea.
 https://gist.github.com/pivotal-jbarrett/beed5f70c1f3a238cef94832b13dab
>> 7a
 
 The biggest issue with the value model is that we have been using a
>> factory
 to build the Cache object. We really don't need one and if we get rid
>> of it
 things look much better. They aren't perfect since we still need the
>> back
 pointer from the impl to the cache for later use. If we didn't need that
 then we could allow copy construction. As it stands right now this
>> version
 allows value, shared_prt, unique_ptr, etc. without any real overhead or
>> RVO
 issues.
 
 The big change is that, rather than have a factory that we set a bunch
>> of
 values on and then ask it to create the Cache, we create a CacheConfig
 object and pass that in to the Cache's constructor. Cache passes it to
 CacheImpl and CacheImpl sets itself up based on the config. If you look
>> at
 what the current factory model does it isn't that different. For
>> clarity I
 added an XmlCacheConfig object to that builds up the CacheConfig via
>> Xml.
 You could imagine a YamlCacheConfig object *shiver*. The point is we
>> don't
 care as long as we get a CacheConfig with all the things we support at
 "init" time.
 
 I know it is a more radical change but I feel it is more C++ and more
 testable than the factory model. I also like that it solves some of the
 issues with the value model we were looking at.
 
 -Jake
 
 On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett 
>> wrote:
 
> Y'all here is an attempt to get the best of both worlds.
> https://gist.github.com/pivotal-jbarrett/
>> 52ba9ec5de0b494368d1c5282ef188
 ef
> 
> I thought I would try posting to Gist but so far not impressed, sorry.
> 
> The Gist of it is that we can overcome the thirdpary or transient
> reference back to the public Cache instance by keeping a reference to
>> it
 in
> the implementation instance and updating it whenever the move
>> constructor
> is called.
> 
> The downside is if your run this test it doesn't show RVO kicking in on
> the second test where we move the value into a shared pointer. There
>> are
 a
> couple of pitfalls you can stumble into as well by trying to used the
> previous instance to access the cache after the move operation, as
> illustrated by the "BAD" commented lines.
> 
> The upside is the choices this gives the use for ownership of their
> factory constructed Cache instance. They can keep it a value or move it
 to
> unique or shared pointer.
> 
> Overhead wise I think we better off in value as long as there are no
> moves, rare I would thing, 

Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread David Kimura
The benefit I see isn't for performance, it's flexibility and ease of use
by application developer.  Anything we can do to achieve this, I think is a
significant win.

I think the reason for the various approaches is to determine whether we
can safely create the simple/flexible API without incurring a penalty on
performance.  Personally, I think the no nullptr benefit that Sarge
mentioned in previous thread is enough to warrant this approach serious
thought even though it provides no performance benefit.

Thanks,
David

On Mon, Sep 18, 2017 at 10:47 AM, Mark Hanson  wrote:

> Hi All,
>
> As we are creating a user API, unless there is a significant performance
> benefit, I think we should try to take the simpler route.
>
> I see benefit to the approach being proposed, but I don’t see a
> significant benefit. Someone please school me if I am wrong.
>
> I am still in the shared pointer camp for cache and a raw pointer to cache
> in cacheImpl.
>
> Sorry, and thanks,
> Mark
>
>
>
> > On Sep 18, 2017, at 10:14 AM, David Kimura  wrote:
> >
> > +1 value approach (via some implementation from this thread)
> >
> > I think I like this.
> >
> > In all BAD cases, it's the user who shot themselves in the foot by using
> > std::move unsafely.  I expect this behavior is the same behavior as for
> any
> > other object.  And if we're ever able to get rid of the Cache/CacheImpl
> > circular dependency then we can add a copy constructor.
> >
> > I also like the idea of passing in a CacheConfig.  My concern though is
> > that it's piling on another big change.
> >
> > Thanks,
> > David
> >
> > On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett 
> wrote:
> >
> >> Ok, one more idea.
> >> https://gist.github.com/pivotal-jbarrett/beed5f70c1f3a238cef94832b13dab
> 7a
> >>
> >> The biggest issue with the value model is that we have been using a
> factory
> >> to build the Cache object. We really don't need one and if we get rid
> of it
> >> things look much better. They aren't perfect since we still need the
> back
> >> pointer from the impl to the cache for later use. If we didn't need that
> >> then we could allow copy construction. As it stands right now this
> version
> >> allows value, shared_prt, unique_ptr, etc. without any real overhead or
> RVO
> >> issues.
> >>
> >> The big change is that, rather than have a factory that we set a bunch
> of
> >> values on and then ask it to create the Cache, we create a CacheConfig
> >> object and pass that in to the Cache's constructor. Cache passes it to
> >> CacheImpl and CacheImpl sets itself up based on the config. If you look
> at
> >> what the current factory model does it isn't that different. For
> clarity I
> >> added an XmlCacheConfig object to that builds up the CacheConfig via
> Xml.
> >> You could imagine a YamlCacheConfig object *shiver*. The point is we
> don't
> >> care as long as we get a CacheConfig with all the things we support at
> >> "init" time.
> >>
> >> I know it is a more radical change but I feel it is more C++ and more
> >> testable than the factory model. I also like that it solves some of the
> >> issues with the value model we were looking at.
> >>
> >> -Jake
> >>
> >> On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett 
> wrote:
> >>
> >>> Y'all here is an attempt to get the best of both worlds.
> >>> https://gist.github.com/pivotal-jbarrett/
> 52ba9ec5de0b494368d1c5282ef188
> >> ef
> >>>
> >>> I thought I would try posting to Gist but so far not impressed, sorry.
> >>>
> >>> The Gist of it is that we can overcome the thirdpary or transient
> >>> reference back to the public Cache instance by keeping a reference to
> it
> >> in
> >>> the implementation instance and updating it whenever the move
> constructor
> >>> is called.
> >>>
> >>> The downside is if your run this test it doesn't show RVO kicking in on
> >>> the second test where we move the value into a shared pointer. There
> are
> >> a
> >>> couple of pitfalls you can stumble into as well by trying to used the
> >>> previous instance to access the cache after the move operation, as
> >>> illustrated by the "BAD" commented lines.
> >>>
> >>> The upside is the choices this gives the use for ownership of their
> >>> factory constructed Cache instance. They can keep it a value or move it
> >> to
> >>> unique or shared pointer.
> >>>
> >>> Overhead wise I think we better off in value as long as there are no
> >>> moves, rare I would thing, but the moves are pretty cheap at the point
> >>> since we only maintain a unique_ptr. After moving into a shared_ptr it
> >> acts
> >>> identical to the shared_ptr model proposed earlier.
> >>>
> >>> -Jake
> >>>
> >>>
> >>> On Thu, Sep 14, 2017 at 3:36 PM Michael Martell 
> >>> wrote:
> >>>
>  Late to this party.
> 
>  Confession 1: I had to look up both RVO and copy-elision.
>  Confession 2: I don't like using pointers at all. I used to, but I've
>  

Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread Mark Hanson
Hi All,

As we are creating a user API, unless there is a significant performance 
benefit, I think we should try to take the simpler route.

I see benefit to the approach being proposed, but I don’t see a significant 
benefit. Someone please school me if I am wrong.

I am still in the shared pointer camp for cache and a raw pointer to cache in 
cacheImpl.

Sorry, and thanks,
Mark



> On Sep 18, 2017, at 10:14 AM, David Kimura  wrote:
> 
> +1 value approach (via some implementation from this thread)
> 
> I think I like this.
> 
> In all BAD cases, it's the user who shot themselves in the foot by using
> std::move unsafely.  I expect this behavior is the same behavior as for any
> other object.  And if we're ever able to get rid of the Cache/CacheImpl
> circular dependency then we can add a copy constructor.
> 
> I also like the idea of passing in a CacheConfig.  My concern though is
> that it's piling on another big change.
> 
> Thanks,
> David
> 
> On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett  wrote:
> 
>> Ok, one more idea.
>> https://gist.github.com/pivotal-jbarrett/beed5f70c1f3a238cef94832b13dab7a
>> 
>> The biggest issue with the value model is that we have been using a factory
>> to build the Cache object. We really don't need one and if we get rid of it
>> things look much better. They aren't perfect since we still need the back
>> pointer from the impl to the cache for later use. If we didn't need that
>> then we could allow copy construction. As it stands right now this version
>> allows value, shared_prt, unique_ptr, etc. without any real overhead or RVO
>> issues.
>> 
>> The big change is that, rather than have a factory that we set a bunch of
>> values on and then ask it to create the Cache, we create a CacheConfig
>> object and pass that in to the Cache's constructor. Cache passes it to
>> CacheImpl and CacheImpl sets itself up based on the config. If you look at
>> what the current factory model does it isn't that different. For clarity I
>> added an XmlCacheConfig object to that builds up the CacheConfig via Xml.
>> You could imagine a YamlCacheConfig object *shiver*. The point is we don't
>> care as long as we get a CacheConfig with all the things we support at
>> "init" time.
>> 
>> I know it is a more radical change but I feel it is more C++ and more
>> testable than the factory model. I also like that it solves some of the
>> issues with the value model we were looking at.
>> 
>> -Jake
>> 
>> On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett  wrote:
>> 
>>> Y'all here is an attempt to get the best of both worlds.
>>> https://gist.github.com/pivotal-jbarrett/52ba9ec5de0b494368d1c5282ef188
>> ef
>>> 
>>> I thought I would try posting to Gist but so far not impressed, sorry.
>>> 
>>> The Gist of it is that we can overcome the thirdpary or transient
>>> reference back to the public Cache instance by keeping a reference to it
>> in
>>> the implementation instance and updating it whenever the move constructor
>>> is called.
>>> 
>>> The downside is if your run this test it doesn't show RVO kicking in on
>>> the second test where we move the value into a shared pointer. There are
>> a
>>> couple of pitfalls you can stumble into as well by trying to used the
>>> previous instance to access the cache after the move operation, as
>>> illustrated by the "BAD" commented lines.
>>> 
>>> The upside is the choices this gives the use for ownership of their
>>> factory constructed Cache instance. They can keep it a value or move it
>> to
>>> unique or shared pointer.
>>> 
>>> Overhead wise I think we better off in value as long as there are no
>>> moves, rare I would thing, but the moves are pretty cheap at the point
>>> since we only maintain a unique_ptr. After moving into a shared_ptr it
>> acts
>>> identical to the shared_ptr model proposed earlier.
>>> 
>>> -Jake
>>> 
>>> 
>>> On Thu, Sep 14, 2017 at 3:36 PM Michael Martell 
>>> wrote:
>>> 
 Late to this party.
 
 Confession 1: I had to look up both RVO and copy-elision.
 Confession 2: I don't like using pointers at all. I used to, but I've
 evolved to just use C# and Java :)
 
 Without investing a lot more time, I don't have strong feelings about
>> raw
 vs shared pointers. My only question is: Can we return ptr to abstract
 class everywhere we return objects? Just thinking of mocking, which
>> always
 wants to mock interfaces.
 
 On Thu, Sep 14, 2017 at 2:25 PM, Michael William Dodge <
>> mdo...@pivotal.io
> 
 wrote:
 
> +0 shared pointer
> 
>> On 14 Sep, 2017, at 14:09, Ernest Burghardt 
> wrote:
>> 
>> Calling a vote for:
>> 
>> - Raw pointer
>> - shard pointer
>> 
>> +1 raw Pointer, I had to look up RVO and am new to std::move(s)
>> 
>> On Thu, Sep 14, 2017 at 3:02 PM, Michael William Dodge <
> mdo...@pivotal.io>
>> 

Re: INFRA Jira URL

2017-09-18 Thread Joey McAllister
I think this is it:
https://issues.apache.org/jira/secure/RapidBoard.jspa?rapidView=25=INFRA.
(At least, that's the JIRA link I got from Infra's contact page.)

On Mon, Sep 18, 2017 at 10:17 AM Kirk Lund  wrote:

> Does anyone know the URL for Apache INFRA Jira?
>
> Thanks,
> Kirk
>


INFRA Jira URL

2017-09-18 Thread Kirk Lund
Does anyone know the URL for Apache INFRA Jira?

Thanks,
Kirk


Re: [Discuss] Investigation of C++ object return types

2017-09-18 Thread David Kimura
+1 value approach (via some implementation from this thread)

I think I like this.

In all BAD cases, it's the user who shot themselves in the foot by using
std::move unsafely.  I expect this behavior is the same behavior as for any
other object.  And if we're ever able to get rid of the Cache/CacheImpl
circular dependency then we can add a copy constructor.

I also like the idea of passing in a CacheConfig.  My concern though is
that it's piling on another big change.

Thanks,
David

On Sun, Sep 17, 2017 at 12:02 AM, Jacob Barrett  wrote:

> Ok, one more idea.
> https://gist.github.com/pivotal-jbarrett/beed5f70c1f3a238cef94832b13dab7a
>
> The biggest issue with the value model is that we have been using a factory
> to build the Cache object. We really don't need one and if we get rid of it
> things look much better. They aren't perfect since we still need the back
> pointer from the impl to the cache for later use. If we didn't need that
> then we could allow copy construction. As it stands right now this version
> allows value, shared_prt, unique_ptr, etc. without any real overhead or RVO
> issues.
>
> The big change is that, rather than have a factory that we set a bunch of
> values on and then ask it to create the Cache, we create a CacheConfig
> object and pass that in to the Cache's constructor. Cache passes it to
> CacheImpl and CacheImpl sets itself up based on the config. If you look at
> what the current factory model does it isn't that different. For clarity I
> added an XmlCacheConfig object to that builds up the CacheConfig via Xml.
> You could imagine a YamlCacheConfig object *shiver*. The point is we don't
> care as long as we get a CacheConfig with all the things we support at
> "init" time.
>
> I know it is a more radical change but I feel it is more C++ and more
> testable than the factory model. I also like that it solves some of the
> issues with the value model we were looking at.
>
> -Jake
>
> On Thu, Sep 14, 2017 at 5:16 PM Jacob Barrett  wrote:
>
> > Y'all here is an attempt to get the best of both worlds.
> > https://gist.github.com/pivotal-jbarrett/52ba9ec5de0b494368d1c5282ef188
> ef
> >
> > I thought I would try posting to Gist but so far not impressed, sorry.
> >
> > The Gist of it is that we can overcome the thirdpary or transient
> > reference back to the public Cache instance by keeping a reference to it
> in
> > the implementation instance and updating it whenever the move constructor
> > is called.
> >
> > The downside is if your run this test it doesn't show RVO kicking in on
> > the second test where we move the value into a shared pointer. There are
> a
> > couple of pitfalls you can stumble into as well by trying to used the
> > previous instance to access the cache after the move operation, as
> > illustrated by the "BAD" commented lines.
> >
> > The upside is the choices this gives the use for ownership of their
> > factory constructed Cache instance. They can keep it a value or move it
> to
> > unique or shared pointer.
> >
> > Overhead wise I think we better off in value as long as there are no
> > moves, rare I would thing, but the moves are pretty cheap at the point
> > since we only maintain a unique_ptr. After moving into a shared_ptr it
> acts
> > identical to the shared_ptr model proposed earlier.
> >
> > -Jake
> >
> >
> > On Thu, Sep 14, 2017 at 3:36 PM Michael Martell 
> > wrote:
> >
> >> Late to this party.
> >>
> >> Confession 1: I had to look up both RVO and copy-elision.
> >> Confession 2: I don't like using pointers at all. I used to, but I've
> >> evolved to just use C# and Java :)
> >>
> >> Without investing a lot more time, I don't have strong feelings about
> raw
> >> vs shared pointers. My only question is: Can we return ptr to abstract
> >> class everywhere we return objects? Just thinking of mocking, which
> always
> >> wants to mock interfaces.
> >>
> >> On Thu, Sep 14, 2017 at 2:25 PM, Michael William Dodge <
> mdo...@pivotal.io
> >> >
> >> wrote:
> >>
> >> > +0 shared pointer
> >> >
> >> > > On 14 Sep, 2017, at 14:09, Ernest Burghardt 
> >> > wrote:
> >> > >
> >> > > Calling a vote for:
> >> > >
> >> > > - Raw pointer
> >> > > - shard pointer
> >> > >
> >> > > +1 raw Pointer, I had to look up RVO and am new to std::move(s)
> >> > >
> >> > > On Thu, Sep 14, 2017 at 3:02 PM, Michael William Dodge <
> >> > mdo...@pivotal.io>
> >> > > wrote:
> >> > >
> >> > >> I generally dig reference-counted pointers for avoiding lifetime
> >> issues
> >> > >> with objects allocated off the heap but I can live with bare
> >> pointers,
> >> > too.
> >> > >>
> >> > >> Sarge
> >> > >>
> >> > >>> On 13 Sep, 2017, at 16:25, Mark Hanson 
> wrote:
> >> > >>>
> >> > >>> Hi All,
> >> > >>>
> >> > >>> I favor the “pointer" approach that is identified in the code
> >> sample.
> >> > >> There is greater clarity and less bytes seemingly created and
> >> written.
> >> > 

Re: How gitbox works

2017-09-18 Thread Kirk Lund
Yeah, I followed all of the directions shared by Jens and Bruce. I also had
my github account added to my Apache account for the last 2-3 years.
Apparently the switchover to gitbox removed by commit permission.

/Users/klund/dev/geode [525]$ git push
ERROR: Permission to apache/geode.git denied to kirklund.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

I also don't have the "Merge" button available to me when viewing Apache
Geode pull requests.

Anyone know who to contact to restore it?

Thanks,
Kirk

On Mon, Sep 18, 2017 at 10:05 AM, Kirk Lund  wrote:

> Apparently I no longer have commit access. Anyone know who/how I get
> access restored?
>
> Thanks,
> Kirk
>
> On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett 
> wrote:
>
>> GitBox is just a fancy name Apache has given to their service that allows
>> us to use GitHub as the source of authority for our Git repo. Everything is
>> done on GitHub using the GitHub processes.
>>
>> Anyone with write access to GitHub can merge a PR. The PR can be merged
>> from the PR GitHub website.
>>
>> -Jake
>>
>>
>> > On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
>> >
>> > I've searched and not found a good summary on how gitbox works. Anyone
>> know
>> > of a good resource?
>> >
>> > The kinds of questions I have are: if other devs approve my PR, do I
>> still
>> > need to find another dev to commit for me or can I commit my own PR if
>> has
>> > been approved?
>> >
>> > Thanks,
>> > Kirk
>>
>
>


Re: How gitbox works

2017-09-18 Thread Kirk Lund
Apparently I no longer have commit access. Anyone know who/how I get access
restored?

Thanks,
Kirk

On Mon, Sep 18, 2017 at 9:49 AM, Jacob Barrett  wrote:

> GitBox is just a fancy name Apache has given to their service that allows
> us to use GitHub as the source of authority for our Git repo. Everything is
> done on GitHub using the GitHub processes.
>
> Anyone with write access to GitHub can merge a PR. The PR can be merged
> from the PR GitHub website.
>
> -Jake
>
>
> > On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
> >
> > I've searched and not found a good summary on how gitbox works. Anyone
> know
> > of a good resource?
> >
> > The kinds of questions I have are: if other devs approve my PR, do I
> still
> > need to find another dev to commit for me or can I commit my own PR if
> has
> > been approved?
> >
> > Thanks,
> > Kirk
>


Re: Request: Build flag to skip download of previous Geode versions

2017-09-18 Thread Jared Stewart
I think this might do it: 

./gradlew build -x :geode-old-versions:compileJava -x 
:geode-old-versions:verifyGeodetest120 -x 
:geode-old-versions:downloadZipFiletest120 -x 
:geode-old-versions:downloadAndUnzipFiletest120 

> On Sep 18, 2017, at 9:46 AM, Kirk Lund  wrote:
> 
> I filed GEODE-3639: "Request: build flag to skip download of previous Geode
> versions" for this request.
> 
> Thanks,
> Kirk
> 
> On Mon, Sep 18, 2017 at 9:40 AM, Kirk Lund  wrote:
> 
>> Turns out the 13% has nothing to do with the download, but I still think a
>> build flag to skip this would be a good idea.
>> 
>> On Mon, Sep 18, 2017 at 9:38 AM, Kirk Lund  wrote:
>> 
>>> So my build has been stuck on our favorite download for the last 10
>>> minutes and is only up to 13% which tells me it could take 20+ minutes this
>>> morning...
>>> 
>>> Download https://www.apache.org/dyn/closer.cgi?action=download
>>> me=geode/1.2.0/apache-geode-1.2.0.tar.gz
>>> <=> 13% EXECUTING
 :geode-old-versions:downloadZipFiletest120 > 38.86 MB/87.14 MB
>>> downloaded
>>> 
>>> What's I'd really like is a build flag to SKIP this step. Is anyone's
>>> Gradle-fu good enough to add in a build flag that would accomplish this?
>>> 
>>> Thanks,
>>> Kirk
>>> 
>> 
>> 



Re: How gitbox works

2017-09-18 Thread Jacob Barrett
GitBox is just a fancy name Apache has given to their service that allows us to 
use GitHub as the source of authority for our Git repo. Everything is done on 
GitHub using the GitHub processes. 

Anyone with write access to GitHub can merge a PR. The PR can be merged from 
the PR GitHub website.

-Jake


> On Sep 18, 2017, at 9:43 AM, Kirk Lund  wrote:
> 
> I've searched and not found a good summary on how gitbox works. Anyone know
> of a good resource?
> 
> The kinds of questions I have are: if other devs approve my PR, do I still
> need to find another dev to commit for me or can I commit my own PR if has
> been approved?
> 
> Thanks,
> Kirk


Re: Request: Build flag to skip download of previous Geode versions

2017-09-18 Thread Kirk Lund
I filed GEODE-3639: "Request: build flag to skip download of previous Geode
versions" for this request.

Thanks,
Kirk

On Mon, Sep 18, 2017 at 9:40 AM, Kirk Lund  wrote:

> Turns out the 13% has nothing to do with the download, but I still think a
> build flag to skip this would be a good idea.
>
> On Mon, Sep 18, 2017 at 9:38 AM, Kirk Lund  wrote:
>
>> So my build has been stuck on our favorite download for the last 10
>> minutes and is only up to 13% which tells me it could take 20+ minutes this
>> morning...
>>
>> Download https://www.apache.org/dyn/closer.cgi?action=download
>> me=geode/1.2.0/apache-geode-1.2.0.tar.gz
>> <=> 13% EXECUTING
>> > :geode-old-versions:downloadZipFiletest120 > 38.86 MB/87.14 MB
>> downloaded
>>
>> What's I'd really like is a build flag to SKIP this step. Is anyone's
>> Gradle-fu good enough to add in a build flag that would accomplish this?
>>
>> Thanks,
>> Kirk
>>
>
>


How gitbox works

2017-09-18 Thread Kirk Lund
I've searched and not found a good summary on how gitbox works. Anyone know
of a good resource?

The kinds of questions I have are: if other devs approve my PR, do I still
need to find another dev to commit for me or can I commit my own PR if has
been approved?

Thanks,
Kirk


Re: Request: Build flag to skip download of previous Geode versions

2017-09-18 Thread Kirk Lund
Turns out the 13% has nothing to do with the download, but I still think a
build flag to skip this would be a good idea.

On Mon, Sep 18, 2017 at 9:38 AM, Kirk Lund  wrote:

> So my build has been stuck on our favorite download for the last 10
> minutes and is only up to 13% which tells me it could take 20+ minutes this
> morning...
>
> Download https://www.apache.org/dyn/closer.cgi?action=download;
> filename=geode/1.2.0/apache-geode-1.2.0.tar.gz
> <=> 13% EXECUTING
> > :geode-old-versions:downloadZipFiletest120 > 38.86 MB/87.14 MB
> downloaded
>
> What's I'd really like is a build flag to SKIP this step. Is anyone's
> Gradle-fu good enough to add in a build flag that would accomplish this?
>
> Thanks,
> Kirk
>


Request: Build flag to skip download of previous Geode versions

2017-09-18 Thread Kirk Lund
So my build has been stuck on our favorite download for the last 10 minutes
and is only up to 13% which tells me it could take 20+ minutes this
morning...

Download
https://www.apache.org/dyn/closer.cgi?action=download=geode/1.2.0/apache-geode-1.2.0.tar.gz
<=> 13% EXECUTING
> :geode-old-versions:downloadZipFiletest120 > 38.86 MB/87.14 MB downloaded

What's I'd really like is a build flag to SKIP this step. Is anyone's
Gradle-fu good enough to add in a build flag that would accomplish this?

Thanks,
Kirk


Build failed in Jenkins: Geode-nightly-flaky #125

2017-09-18 Thread Apache Jenkins Server
See 

--
Started by upstream project "Geode-nightly" build number 958
originally caused by:
 Started by timer
[EnvInject] - Loading node environment variables.
Building remotely on H14 (couchdbtest ubuntu xenial) in workspace 

 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://github.com/apache/geode # timeout=10
Fetching upstream changes from https://github.com/apache/geode
 > git --version # timeout=10
 > git fetch --tags --progress https://github.com/apache/geode 
 > +refs/heads/*:refs/remotes/origin/*
ERROR: Error fetching remote repo 'origin'
hudson.plugins.git.GitException: Failed to fetch from 
https://github.com/apache/geode
at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:817)
at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1084)
at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1115)
at hudson.scm.SCM.checkout(SCM.java:495)
at hudson.model.AbstractProject.checkout(AbstractProject.java:1276)
at 
hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:560)
at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86)
at 
hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:485)
at hudson.model.Run.execute(Run.java:1735)
at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
at hudson.model.ResourceController.execute(ResourceController.java:97)
at hudson.model.Executor.run(Executor.java:405)
Caused by: hudson.plugins.git.GitException: Command "git fetch --tags 
--progress https://github.com/apache/geode +refs/heads/*:refs/remotes/origin/*" 
returned status code 128:
stdout: 
stderr: remote: Counting objects: 188, done.
remote: Compressing objects:   1% (1/67)   remote: Compressing objects: 
  2% (2/67)   remote: Compressing objects:   4% (3/67)   
remote: Compressing objects:   5% (4/67)   remote: Compressing objects: 
  7% (5/67)   remote: Compressing objects:   8% (6/67)   
remote: Compressing objects:  10% (7/67)   remote: Compressing objects: 
 11% (8/67)   remote: Compressing objects:  13% (9/67)   
remote: Compressing objects:  14% (10/67)   remote: Compressing 
objects:  16% (11/67)   remote: Compressing objects:  17% (12/67)   
remote: Compressing objects:  19% (13/67)   remote: Compressing 
objects:  20% (14/67)   remote: Compressing objects:  22% (15/67)   
remote: Compressing objects:  23% (16/67)   remote: Compressing 
objects:  25% (17/67)   remote: Compressing objects:  26% (18/67)   
remote: Compressing objects:  28% (19/67)   remote: Compressing 
objects:  29% (20/67)   remote: Compressing objects:  31% (21/67)   
remote: Compressing objects:  32% (22/67)   remote: Compressing 
objects:  34% (23/67)   remote: Compressing objects:  35% (24/67)   
remote: Compressing objects:  37% (25/67)   remote: Compressing 
objects:  38% (26/67)   remote: Compressing objects:  40% (27/67)   
remote: Compressing objects:  41% (28/67)   remote: Compressing 
objects:  43% (29/67)   remote: Compressing objects:  44% (30/67)   
remote: Compressing objects:  46% (31/67)   remote: Compressing 
objects:  47% (32/67)   remote: Compressing objects:  49% (33/67)   
remote: Compressing objects:  50% (34/67)   remote: Compressing 
objects:  52% (35/67)   remote: Compressing objects:  53% (36/67)   
remote: Compressing objects:  55% (37/67)   remote: Compressing 
objects:  56% (38/67)   remote: Compressing objects:  58% (39/67)   
remote: Compressing objects:  59% (40/67)   remote: Compressing 
objects:  61% (41/67)   remote: Compressing objects:  62% (42/67)   
remote: Compressing objects:  64% (43/67)   remote: Compressing 
objects:  65% (44/67)   remote: Compressing objects:  67% (45/67)   
remote: Compressing objects:  68% (46/67)   remote: Compressing 
objects:  70% (47/67)   remote: Compressing objects:  71% (48/67)   
remote: Compressing objects:  73% (49/67)   remote: Compressing 
objects:  74% (50/67)   remote: Compressing objects:  76% (51/67)   
remote: Compressing objects:  77% (52/67)   remote: Compressing 
objects:  79% (53/67)   remote: Compressing objects:  80% (54/67)   
remote: Compressing objects:  82% (55/67)   remote: Compressing 
objects:  83% (56/67)   remote: Compressing objects:  85% (57/67)   
remote: 

Build failed in Jenkins: Geode-nightly #958

2017-09-18 Thread Apache Jenkins Server
See 

--
[...truncated 117.85 KB...]
:geode-json:distributedTest NO-SOURCE
:geode-json:integrationTest NO-SOURCE
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
:geode-junit:compileTestJavaNote: 

 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: 

 uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-junit:processTestResources
:geode-junit:testClasses
:geode-junit:checkMissedTests
:geode-junit:spotlessJavaCheck
:geode-junit:spotlessCheck
:geode-junit:test
:geode-junit:check
:geode-junit:build
:geode-junit:distributedTest
:geode-junit:integrationTest
:geode-lucene:assemble
:geode-lucene:compileTestJavaNote: Some input files use or override a 
deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-lucene:processTestResources
:geode-lucene:testClasses
:geode-lucene:checkMissedTests
:geode-lucene:spotlessJavaCheck
:geode-lucene:spotlessCheck
:geode-lucene:test
:geode-lucene:check
:geode-lucene:build
:geode-lucene:distributedTest
:geode-lucene:integrationTest
:geode-old-client-support:assemble
:geode-old-client-support:compileTestJavaNote: 

 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:geode-old-client-support:processTestResources NO-SOURCE
:geode-old-client-support:testClasses
:geode-old-client-support:checkMissedTests
:geode-old-client-support:spotlessJavaCheck
:geode-old-client-support:spotlessCheck
:geode-old-client-support:test
:geode-old-client-support:check
:geode-old-client-support:build
:geode-old-client-support:distributedTest
:geode-old-client-support:integrationTest
:geode-old-versions:distributedTest NO-SOURCE
:geode-old-versions:integrationTest NO-SOURCE
:geode-protobuf:assemble
:geode-protobuf:extractIncludeTestProto
:geode-protobuf:extractTestProto UP-TO-DATE
:geode-protobuf:generateTestProto NO-SOURCE
:geode-protobuf:compileTestJavaNote: Some input files use unchecked or unsafe 
operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-protobuf:processTestResources
:geode-protobuf:testClasses
:geode-protobuf:checkMissedTests
:geode-protobuf:spotlessJavaCheck
:geode-protobuf:spotlessCheck
:geode-protobuf:test
:geode-protobuf:check
:geode-protobuf:build
:geode-protobuf:distributedTest
:geode-protobuf:integrationTest

org.apache.geode.protocol.acceptance.CacheConnectionTimeoutJUnitTest > 
testUnresponsiveClientsGetDisconnected FAILED
org.awaitility.core.ConditionTimeoutException: Condition defined as a 
lambda expression in 
org.apache.geode.protocol.acceptance.CacheConnectionTimeoutJUnitTest null 
within 1120 milliseconds.
at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:104)
at 
org.awaitility.core.AssertionCondition.await(AssertionCondition.java:117)
at 
org.awaitility.core.AssertionCondition.await(AssertionCondition.java:32)
at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:809)
at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:648)
at 
org.apache.geode.protocol.acceptance.CacheConnectionTimeoutJUnitTest.testUnresponsiveClientsGetDisconnected(CacheConnectionTimeoutJUnitTest.java:128)

13 tests completed, 1 failed
:geode-protobuf:integrationTest FAILED
:geode-pulse:assemble
:geode-pulse:compileTestJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: 

 uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-pulse:processTestResources
:geode-pulse:testClasses
:geode-pulse:checkMissedTests
:geode-pulse:spotlessJavaCheck
:geode-pulse:spotlessCheck
:geode-pulse:test
:geode-pulse:check
:geode-pulse:build
:geode-pulse:distributedTest
:geode-pulse:integrationTest
:geode-rebalancer:assemble
:geode-rebalancer:compileTestJava
:geode-rebalancer:processTestResources NO-SOURCE
:geode-rebalancer:testClasses
:geode-rebalancer:checkMissedTests
:geode-rebalancer:spotlessJavaCheck
:geode-rebalancer:spotlessCheck
:geode-rebalancer:test
:geode-rebalancer:check
:geode-rebalancer:build