Add docs about using ADAM's Kryo registrator from another Kryo registrator. #1305

Merged
merged 1 commit into from Dec 8, 2016

Conversation

Projects
None yet
3 participants
@fnothaft
Member

fnothaft commented Dec 8, 2016

Follow on from @heuermh's #1304.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Dec 8, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1665/
Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1665/
Test PASSed.

docs/source/60_building_apps.md
+The following serialization configuration needs to be present to register ADAM classes. This
+assumes that you do not need to register additional custom
+[Kryo Serializers](https://github.com/EsotericSoftware/kryo). If you do, you will need to
+[write your own registrator that calls the ADAM registrator](#registrator).

This comment has been minimized.

@heuermh

heuermh Dec 8, 2016

Member

[Kyro Serializers][Kyro serializers]

Would this read better as:

The following serialization configuration needs to be present to register ADAM classes. If any additional Kyro serializers need to be registered, (...and why would they need to be?...), create a registrator that delegates to the ADAM registrator.

@heuermh

heuermh Dec 8, 2016

Member

[Kyro Serializers][Kyro serializers]

Would this read better as:

The following serialization configuration needs to be present to register ADAM classes. If any additional Kyro serializers need to be registered, (...and why would they need to be?...), create a registrator that delegates to the ADAM registrator.

docs/source/60_building_apps.md
+
+### Writing your own registrator that calls the ADAM registrator {#registrator}
+
+As we do in ADAM, you may want to provide your own Kryo serializer registrator,

This comment has been minimized.

@heuermh

heuermh Dec 8, 2016

Member

I hadn't been using "you" in this doc, so this paragraph doesn't use the same voice as above. I guess I didn't check the other doc pages to see.

@heuermh

heuermh Dec 8, 2016

Member

I hadn't been using "you" in this doc, so this paragraph doesn't use the same voice as above. I guess I didn't check the other doc pages to see.

docs/source/60_building_apps.md
+registrator from within your registrator. To do this, you would write code
+like this:
+
+```

This comment has been minimized.

@heuermh

heuermh Dec 8, 2016

Member

Use ```scala for syntax highlighting

@heuermh

heuermh Dec 8, 2016

Member

Use ```scala for syntax highlighting

docs/source/60_building_apps.md
+import org.apache.spark.serializer.KryoRegistrator
+import org.bdgenomics.adam.serialization.ADAMKryoRegistrator
+
+class AvocadoKryoRegistrator extends KryoRegistrator {

This comment has been minimized.

@heuermh

heuermh Dec 8, 2016

Member

AvocadoKryoRegistratorMyCommandKyroRegistrator to match example code above

@heuermh

heuermh Dec 8, 2016

Member

AvocadoKryoRegistratorMyCommandKyroRegistrator to match example code above

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Dec 8, 2016

Member

Thanks for the review @heuermh! I've rolled your suggestions in.

Member

fnothaft commented Dec 8, 2016

Thanks for the review @heuermh! I've rolled your suggestions in.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Dec 8, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1666/
Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1666/
Test PASSed.

@heuermh

heuermh approved these changes Dec 8, 2016

@heuermh heuermh merged commit 518106e into bigdatagenomics:master Dec 8, 2016

1 check passed

default Merged build finished.
Details
@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Dec 8, 2016

Member

Looks great, thank you, @fnothaft

Member

heuermh commented Dec 8, 2016

Looks great, thank you, @fnothaft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment