I wonder if Kirk's and Naba's statements are necessarily at odds.
Make the change easy (warning: this may be hard), then make the easy
> change."
-Kent Beck
Following Kent Beck's advise might reasonably split into two commits. One
refactor commit and a separate commit that introduces the
It is to help with bisect operations when things start failing ... helps us
it revert and build faster.
also with cherry picking features / fixes to previous versions .
And keeping the git history clean with no unnecessary “merge commits”.
Regards
Naba
On Fri, Dec 13, 2019 at 2:25 PM Kirk Lund
-1 I really like to sometimes have more than 1 commit in a PR and keep them
separate when they merge to develop
On Tue, Oct 22, 2019 at 5:12 PM Nabarun Nag wrote:
> Hi Geode Committers,
>
> A kind request for using squash commit instead of using merge.
> This will really help us in our bisect
Hi Mario,
I think I see what is going on here. The logic for "reindex" code was a
bit off ( it expected reindex features to be complete by a certain
release). I have a PR on develop to adjust that calculation (
https://github.com/apache/geode/pull/4466)
The expectation is that when lucene
+1
> On Oct 28, 2019, at 12:47 PM, Jacob Barrett wrote:
>
> +1
>
>
>> On Oct 22, 2019, at 5:12 PM, Nabarun Nag wrote:
>>
>> Hi Geode Committers,
>>
>> A kind request for using squash commit instead of using merge.
>> This will really help us in our bisect operations when a
>>
We could have the content of the PR jobs check that actual sources changed,
before doing any real 'work' or 'validation' of the change. If the patch is
only CI or readme (not docs, those get compiled) then we can exit with a
clean state. Saves time, and money!
If the issue is the lack of
Hi Michael, that does sound like a frustrating experience. Thanks for saying
something — it is harder to track how often this is occurring if people don’t
report the issue on the dev list.
Geode’s PR pipeline uses https://hub.docker.com/r/teliaoss/github-pr-resource
Well lack of review was one part. The major part was that if you click on
the checks from the other empty commits, you'll see that the required
passing jobs never triggered (only the LGTM checks).
-michael
On Fri, Dec 13, 2019 at 11:43 AM Joey McAllister
wrote:
> Hi Michael,
>
> That may have
Hi Michael,
That may have been connected, at least in part, to a lack of a review. I
have given my review on adding this info to the README, which I think looks
good, and things seem to have gone green.
That said, as I mentioned in my review, I also wouldn't mind seeing a
thumbs up (or other
Hi Geode Dev!
This PR https://github.com/apache/geode/pull/4406 is a change to the
readme. However it took 3 empty commits to get it to go green enough to be
allowed to be merged. This seems odd, especially with just a readme
change. Is there something going with how CI works for PRs? This
The v2 management api allows regions to be created remotely in cluster
configuration and on running servers. But it does not allow you to create a
region on a client. Non-spring applications can use RegionFactory to create
the region on the client side.
On Fri, Dec 13, 2019 at 9:56 AM Dan Smith
+1 to adding a way to copy the RegionAttributes.
BTW, I really wish this RegionFactory was an interface. I don't know if
adding a copy constructor makes it harder to migrate to an interface later,
but maybe just having this single public method might be better?
+ RegionFactory
Hi Udo,
I disagree. I believe the currently published best practice is to use
RegionFactory. As a part of the work I have been doing, I have been migrating
code from the AttributesFactory pattern to the RegionFactory pattern. To
support that, I believe a copy constructor for RegionFactory is
Transactions are an area of the codebase I've never dealt with, so I'm sure
most/all of what you mention is true. In particular, the only thing I know
about TransactionId is that it's always set to -1, so functionally it's
essentially useless. I'm not certain what all of the implications will be
Hi,
I'd appreciate some extra reviewer (I already had one, thanks @Dave Barnes) and
if everything is ok someone to merge the following pull request:
https://github.com/apache/geode/pull/4452
Thanks,
Alberto
Hi,
I have created a ticket with some issues I have found related to TransactionIds
managed by CacheTransactionManager in the C++ native client.
https://issues.apache.org/jira/browse/GEODE-7573
In it, I also propose some solutions to the issues found.
I'd appreciate if someone could review
16 matches
Mail list logo
