[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...

2017-07-09 Thread gregt5259 
Github user gregt5259 commented on the issue:

https://github.com/apache/geode-native/pull/106
  
Pull request is closed after passing of all checks and marked as resolved 
in Jira GEODE-2891 with resolution "Won't fix" as following:
GEODE-2891 - connect-timeout violation in C++ Native Client
is superceded by
GEODE-3137 - Replace all time values internally with std::chrono types


---
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 feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...

2017-07-06 Thread gregt5259 
Github user gregt5259 commented on the issue:

https://github.com/apache/geode-native/pull/106
  
In my mind we talk about the some kind of redesign rather than about 
correct change: the original design is based de-facto exactly on using ‘magic 
numbers’; configurable measurements units in an explicit form are missing in 
the original design. Yes, my solution solves the problem in the code 
implementing accordingly to the original design and quite could be realized in 
the code implementing original design without waiting  for future redesigns and 
refactoring, especially with undefined due date.

Thanks,
Dr. Gregory Turovets

"…We're all mad here. I'm mad. You're mad."
"How do you know I'm mad?" said Alice.
"You must be," said the Cat, "or you wouldn't have come here."
Alice's Adventures in Wonderland by Lewis 
Carroll.

From: Jacob Barrett [mailto:notificati...@github.com]
Sent: Thursday, July 06, 2017 17:35
To: apache/geode-native 
Cc: Gregory Turovets ; Mention 

Subject: Re: [apache/geode-native] GEODE-2891 connect-timeout violation in 
C++ Native Client (#106)


@gregt5259 This is a solution to the problem 
but not the solution we as committers are comfortable committing as it directly 
conflicts with the correct change, which is to use type safe durations rather 
than magic number math and system wide properties to create a confusing array 
of time values.

If you want this change sooner than later you could implement it using 
std::chrono::duration as outlined in GEODE-3137 or maintain a fork with your 
change in it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on 
GitHub, 
or mute the 
thread.
This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

you may review at https://www.amdocs.com/about/email-disclaimer 




---
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 feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...

2017-07-06 Thread pivotal-jbarrett 
Github user pivotal-jbarrett commented on the issue:

https://github.com/apache/geode-native/pull/106
  
@gregt5259 This is a solution to the problem but not the solution we as 
committers are comfortable committing as it directly conflicts with the correct 
change, which is to use type safe durations rather than magic number math and 
system wide properties to create a confusing array of time values.

If you want this change sooner than later you could implement it using 
std::chrono::duration as outlined in GEODE-3137 or maintain a fork with your 
change in it.



---
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 feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...

2017-07-06 Thread gregt5259 
Github user gregt5259 commented on the issue:

https://github.com/apache/geode-native/pull/106
  
Did I understood correct that there are no issues found during the code 
review in the reviewed code? Probably the decision regarding the accepting of 
the pull request doesn’t depend in this case on the code quality but should 
depend on ETA for GEODE-3136 
and GEODE-3137, on the 
appropriate next client version deployment readiness et cetera. If these dates 
will be published, that will assist us within the company to take decision 
whether we may wait for this client version or will require to accept the pull 
request even as the temporary fix.

Thanks,
Dr. Gregory Turovets

From: Jacob Barrett [mailto:notificati...@github.com]
Sent: Wednesday, July 05, 2017 17:41
To: apache/geode-native 
Cc: Gregory Turovets ; Author 

Subject: Re: [apache/geode-native] GEODE-2891 connect-timeout violation in 
C++ Native Client (#106)


@pivotal-jbarrett requested changes on this pull request.

I am not in favor of accepting this pull request on the heals of correcting 
all timeouts via GEODE-3136 
and GEODE-3137 as mentioned 
in pull #105.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on 
GitHub,
 or mute the 
thread.
This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

you may review at https://www.amdocs.com/about/email-disclaimer 




---
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 feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...

2017-07-05 Thread echobravopapa 
Github user echobravopapa commented on the issue:

https://github.com/apache/geode-native/pull/106
  
@pivotal-jbarrett fair enough on the branching point, I think the rest is 
still a valid ask


---
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 feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...

2017-07-05 Thread pivotal-jbarrett 
Github user pivotal-jbarrett commented on the issue:

https://github.com/apache/geode-native/pull/106
  
@echobravopapa the branching scheme only applies to committers working 
directly with the Geode repositories. Non-committers use pull requests which 
render the branching/girflow requirement null.


---
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 feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...

2017-07-05 Thread echobravopapa 
Github user echobravopapa commented on the issue:

https://github.com/apache/geode-native/pull/106
  
@gregt5259 a couple quick items of feedback that need to be addressed 
before this is reviewed:

- please squash your commits, this cleans up the diffs for everyone to 
better review your contribution
please close [#105](https://github.com/apache/geode-native/pull/105) as 
duplicate of this PR
- please have a look at the [Code 
Contribution](https://cwiki.apache.org/confluence/display/GEODE/Code+contributions)
 section of the WIKI
-- for one you will see that we are using GitFlow and your PR should be 
coming in from a feature branch (e.g.  feature/GEODE-2891) instead of your 
develop branch

 Thanks!


---
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 feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---