Hi Stian,

Apart from the Graph.add change that is now reverted, the rest was a
combination and extension of your pull requests that had been open and
debated for a long time.

It was clear that we weren't going to stick with the local scope
system that wasn't easy to understand or internally consistent, so I
put your ideas together and reformed it into a package of changes that
we can debate against without having the "localScope" keep coming up.
There is now no communication between implementations other than the
UUID which we had all basically agreed on already, and the mapping is
done consistently within each implementation, including a consistent
mapping across time which is necessary for streaming parsers to
operate in constant memory. Ie, a Sesame parser can stream its Triples
directly into a Jena Model in constant memory based on the UUID giving
non-anonymous (in the concrete syntax) BlankNodes a consistent
identity across time.

In this case, the pull request was open for 24 hours without comments
so, as I only really get time to code on this project on weekends, I
pragmatically merged it:

https://github.com/apache/incubator-commonsrdf/pull/1

I am fine with having people reviewing pull requests by default, or
merging after a reasonable amount of time if noone comments at all (24
hours is probably be too short in general but with the number of
administrative changes Sergio is making to the codebase I didn't want
them to go stale over a week, and with Easter it may have been more
than a week in this case if they were not merged then). As I mentioned
above, in this case the discussion had long been stagnant around your
3-4 alternative pull requests for options to change the
BlankNode.internalIdentifier contract and this combined them into a
single package so we can open the debate up again with a different
perspective.

In terms of the PR opening and discussion emailing notifications here,
that should work in general, but it didn't work in this case. It works
for Any23, so it does work in theory, just not for this project yet.

Cheers,

Peter

On 31 March 2015 at 21:30, Stian Soiland-Reyes <[email protected]> wrote:
> Hi,
>
> I saw this big merge from Peter:
>
> https://github.com/apache/incubator-commonsrdf/commit/20b0b000fc92a5699e0d79f9205cb62babe444bd
>
> This includes not just sorting out some BlankNode identifier stuff
> which we have already discussed at length (good) - but also a few
> other changes that got baked in, e.g. the return type of Graph.add(),
> and the removal of
>
> - Stream<? extends Triple> getTriples(Predicate<Triple> filter
>
>
> While I am not opposed to this removal (it's available as
> getTriples().filter()), I can't remember seeing these API changes
> being discussed (perhaps I missed something) - are we not keeping the
> way of working by pull requests and bugs?
>
>
> I would also prefer separate concerns as separate commits - I know
> from my own way of working that you find one thing when you work with
> another - but say getTriples(Predicate) has nothing with BlankNode
> identifiers directly, and so this should be a separate change request.
>
>
>
> I know we can follow and respond to commits@commonsrdf - but specially
> from git this often comes with 17 emails for a single merge, and so
> it's not good for anything but "something has changed".
>
>
> I propose that we work like this:
>
> API changes (e.g. signature modifications, change of semantics in
> javadoc) always as pull requests and/or Jira issues.
>
> Fixing typos, NullPointerExceptions and so on can just commit straight in.
>
> "Trying out something" should be done as a branch or personal fork.
> (Branch is preferred, as it allows others to modify your experiment)
>
> Semantic changes to the tests should be pull requests and Jira issues
> - as they are the other half of the contract.
>
> Changes to the simple implementation to conform with the tests can go
> straight in.
>
> Changes to the simple implementation that forces a new contract
> implication (typically something we haven't explored in detail yet)
> should be formalized as new javadoc/tests - and therefore raised as a
> separate discussion point by pull request or issue.
>
> Brand new implementation stuff should be done as a branch or pull
> request (there is probably not many of these for now).
>
>
> In general - a pull request is preferable to an issue, as an issue is
> an unbounded discussion, while a pull request is a concrete change
> request that can be tweaked.
>
> If we do it the Commons way - then any other committer can review and
> accept the pull request - although I think we should also allow room
> for the consensus to form - specially on API changes & design issues
> which we want to be considerate about.
>
>
> Views?
>
> --
> Stian Soiland-Reyes
> Apache Taverna (incubating), Apache Commons RDF (incubating)
> http://orcid.org/0000-0001-9842-9718

Reply via email to