Skip to content
This repository has been archived by the owner on Oct 28, 2022. It is now read-only.

Move models and RPC methods out of root package #209

Merged
merged 1 commit into from
Dec 17, 2014

Conversation

massie
Copy link
Member

@massie massie commented Dec 5, 2014

This updates the namespace to place the models into the
org.ga4gh.models package and RPC methods into the
org.ga4gh.rpc package. Previously, all objects were on
the root package org.ga4gh.

@fnothaft
Copy link
Contributor

fnothaft commented Dec 5, 2014

+1

@diekhans
Copy link
Contributor

diekhans commented Dec 5, 2014

big +1

Matt Massie notifications@github.com writes:

This updates the namespace to place the models into the
org.ga4gh.models package and RPC methods into the
org.ga4gh.rpc package. Previously, all objects were on
the root package org.ga4gh.

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

You can merge this Pull Request by running

git pull https://github.com/massie/schemas namespace

Or view, comment on, or merge it at:

#209

Commit Summary

• Move models and RPC methods out of root package

File Changes

• M .gitignore (3)
• M GeneratingDocumentation.md (6)
• M src/main/resources/avro/beacon.avdl (2)
• M src/main/resources/avro/common.avdl (13)
• M src/main/resources/avro/readmethods.avdl (13)
• M src/main/resources/avro/reads.avdl (2)
• M src/main/resources/avro/referencemethods.avdl (11)
• M src/main/resources/avro/references.avdl (2)
• A src/main/resources/avro/rpc.avdl (18)
• M src/main/resources/avro/variantmethods.avdl (11)
• M src/main/resources/avro/variants.avdl (2)
• M src/main/resources/avro/wip/metadata.avdl (2)
• M src/main/resources/avro/wip/metadatamethods.avdl (13)

Patch Links:

https://github.com/ga4gh/schemas/pull/209.patch
https://github.com/ga4gh/schemas/pull/209.diff


Reply to this email directly or view it on GitHub.*

@benedictpaten
Copy link
Contributor

+1

On Fri, Dec 5, 2014 at 2:53 PM, Mark Diekhans notifications@github.com
wrote:

big +1

Matt Massie notifications@github.com writes:

This updates the namespace to place the models into the
org.ga4gh.models package and RPC methods into the
org.ga4gh.rpc package. Previously, all objects were on
the root package org.ga4gh.

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

You can merge this Pull Request by running

git pull https://github.com/massie/schemas namespace

Or view, comment on, or merge it at:

#209

Commit Summary

• Move models and RPC methods out of root package

File Changes

• M .gitignore (3)
• M GeneratingDocumentation.md (6)
• M src/main/resources/avro/beacon.avdl (2)
• M src/main/resources/avro/common.avdl (13)
• M src/main/resources/avro/readmethods.avdl (13)
• M src/main/resources/avro/reads.avdl (2)
• M src/main/resources/avro/referencemethods.avdl (11)
• M src/main/resources/avro/references.avdl (2)
• A src/main/resources/avro/rpc.avdl (18)
• M src/main/resources/avro/variantmethods.avdl (11)
• M src/main/resources/avro/variants.avdl (2)
• M src/main/resources/avro/wip/metadata.avdl (2)
• M src/main/resources/avro/wip/metadatamethods.avdl (13)

Patch Links:

https://github.com/ga4gh/schemas/pull/209.patch
https://github.com/ga4gh/schemas/pull/209.diff


Reply to this email directly or view it on GitHub.*


Reply to this email directly or view it on GitHub
#209 (comment).

@jeromekelleher
Copy link
Contributor

I think it's a good move separating the namespace like this, but is rpc the right choice? There's still a lot of people out there for which RPC means something very specific. I bring this up because I've used rpc for something before and some heavy-duty Unix people were very confused about what I was talking about and were generally of the opinion that this is not a good term to reuse.

Also, 'remote procedure call' is a bit more general than what we're actually doing here; we're just exposing methods to access data, rather than providing a general approach to running code remotely.

Since everything within the package is of the form XMethods, perhaps methods would be a better choice for the package name?

@massie
Copy link
Member Author

massie commented Dec 8, 2014

@jeromekelleher I originally used methods but then changed it to rpc. I'm happy to change it back to methods. I'll make that change tomorrow.

(I'm surprised that heavy-duty Unix people were confused what RPC meant since XDR/RPC has been a part of Unix since the 1980s)

@massie
Copy link
Member Author

massie commented Dec 8, 2014

Ah, misread your comment. Make sense they were confused if they thought all RPC is XDR/RPC.

This updates the namespace to place the models into the
org.ga4gh.models package and RPC methods into the
org.ga4gh.rpc package. Previously, all objects were on
the root package org.ga4gh.
@massie
Copy link
Member Author

massie commented Dec 8, 2014

I found a little time now and made the s/rpc/methods/g change. Hopefully, we're all ready to merge now.

@jeromekelleher
Copy link
Contributor

Look great, thanks @massie! +1.

Yeah, they got confused because they thought that the Python 'RPC' layer that I'd written was literally using Sun RPC. With good reason too, as that would be a deeply strange and weird thing to do!

@delagoya
Copy link
Contributor

+1
It has been nine days. Vote to merge before today's call(s). Any objections?

delagoya added a commit that referenced this pull request Dec 17, 2014
Move models and RPC methods out of root package
@delagoya delagoya merged commit 46065e0 into ga4gh:master Dec 17, 2014
delagoya added a commit that referenced this pull request Dec 17, 2014
delagoya added a commit that referenced this pull request Dec 17, 2014
This was a merge of conflicts between PR #209 and #205. Whitespace issues
now integrated with the namespacing of methods and models.
@jeromekelleher jeromekelleher mentioned this pull request Jan 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants