Re: A couple pull requests in the pending tray.

[Andy Seaborne <an...@apache.org>]

On 31/05/15 21:54, Claude Warren wrote:
> Andy,
>
> I did a review and the changes make sense (the structure is a holdover from
> a very early incarnation).  I would do the merge but I don't know.

".. how to do that?"

>
> a) what id to use on github to do the merge? or do I do the merge elsewhere?
> b) I am not certain what actions I would take to do the merge.

You are doing with to bring things into the Apache Jena repo with your 
Apache id.  Not as a guthub person nor with their tools.

There are several ways depending.

1/ If you want to include all the commits, and maybe tweak things after, 
then

git pull https://github.com/apache/jena/pull/NN/head

(caution see (3) I don't do that - I use a short form).
<any tweaks, commit>

# Schedule a close.
git commit -m "This closes #NN." --allow-empty
# Push to Apache.
git push

The "This closes..." can be in any commit message; I've shown how to do 
it if you just pull, make no changes, and want to have the close happen.

Advantage of this route : the commits have the contributors name.

2/ If I'm picking and choosing, i.e. more than a few tweaks, I've found 
it easier to grab some changes and work locally.

Get the diff by downloading as a file:

     https://github.com/apache/jena/pull/NN.diff

(I have found that diff is better than patch when the PR has been 
modified a few times ".patch" don't see to always apply the revisions, 
".diff" is OK)

Apply it
   git apply NN.diff

Work locally, commit changes. Make sure one has "This closes #NN"

3/ I have this in <Jena>/.git/config

------------------------------
[remote "github"]
	url = git@github.com:apache/jena.git
	fetch = +refs/heads/*:refs/remotes/github/*
------------------------------

so it's

git pull github pull/NN/head

	Andy

>
> Claude
>
> On Fri, May 29, 2015 at 5:27 PM, Andy Seaborne <an...@apache.org> wrote:
>
>> Claude, Rob,
>>
>> I took a look at our outstanding pull requests on github.
>>
>> I was wondering if you have a moment to assess them.  It's clearing up
>> suggestions from ajs6f, not bug fixes nor new features.  They were part of
>> large sets of changes over several modules but ajs6f very kindly broken
>> them up in units that reflect the module structure of Jena.
>>
>> Claude:
>>
>> "Nonfunctional cleanup in jena-querybuilder"
>> https://github.com/apache/jena/pull/68
>>
>> the bulk of this is subQuery is known to be instanceof the various cases
>> do the if/instanceof isn't necessary.   Your choice on style.
>>
>>
>> Rob:
>>
>> "Nonfunctional cleanup in jena-jdbc"
>> https://github.com/apache/jena/pull/62
>>
>> Much of this is adjusting the throws clauses on @Override methods.
>> I personally prefer normally to keep the throws even if not actually
>> thrown because they are in the contract of the interface.  I didn't apply
>> similar changes elsewhere.
>>
>>          Andy
>>
>
>
>