Skip to content
This repository was archived by the owner on Oct 29, 2023. It is now read-only.

Conversation

deflaux
Copy link
Contributor

@deflaux deflaux commented Apr 13, 2016

Pipeline-specific functions were pushed down a package level and one reusable function was moved out of an obsolete 'grpc' package level.

@iliat please take a look at the new organization and let me know if any of the BAM stuff should be grouped differently.

https://github.com/deflaux/dataflow-java/tree/rearrange/src/main/java/com/google/cloud/genomics/dataflow/functions

https://github.com/deflaux/dataflow-java/tree/rearrange/src/main/java/com/google/cloud/genomics/dataflow/utils

https://github.com/deflaux/dataflow-java/tree/rearrange/src/main/java/com/google/cloud/genomics/dataflow/writers/bam

@coveralls
Copy link

Coverage Status

Coverage remained the same at 20.263% when pulling efdf35f on deflaux:rearrange into 7d25de2 on googlegenomics:master.

@pgrosu
Copy link

pgrosu commented Apr 13, 2016

I would use more general terms such as analysis, validation or qc to group a larger set of classes under. It would be hard for folks to remember transmissionprob as package names, as in Java either they have the whole name in CamelCase, or is only one common word that is truncated, or has the whole package name as an acronym.

The Java class hierarchy provides a nice idea of how to structure an API:

https://docs.oracle.com/javase/8/docs/api/java/lang/package-tree.html

https://docs.oracle.com/javase/8/docs/api/java/util/package-tree.html

Hope it helps,
~p

@iliat
Copy link
Contributor

iliat commented Apr 13, 2016

LGTM for BAM, Thanks !

@coveralls
Copy link

Coverage Status

Coverage remained the same at 20.263% when pulling 8b06610 on deflaux:rearrange into 7d25de2 on googlegenomics:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 20.263% when pulling 1b92fb6 on deflaux:rearrange into 7d25de2 on googlegenomics:master.

@pgrosu
Copy link

pgrosu commented Apr 14, 2016

The fixes look nice - thanks Nicole :)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 20.263% when pulling 0134a4f on deflaux:rearrange into 7d25de2 on googlegenomics:master.

@deflaux deflaux merged commit 5aa788d into googlegenomics:master Apr 14, 2016
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.

4 participants