Skip to content
This repository has been archived by the owner on Jun 1, 2021. It is now read-only.

Reorganized sample code and began rewriting the User Guide, with changes to affected documents #389

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mslinn
Copy link
Contributor

@mslinn mslinn commented Mar 30, 2017

This is a huge PR. It arose from my attempts to run the code examples in the User Guide. Along the way I realized that most of them in fact did not run, so I raised #384. As I mentioned in the #384 comments, the code examples needed to be reorganized so they could be run.

This PR:

  • Adds material to the User Guide (should be checked for accuracy)
  • Moves code examples to the new top-level examples directory
  • Adjusts every reference to the moved code examples
  • Adds TODO and FIXME comments in broken code examples (remember, these examples were previously broken).
  • All tests pass
  • Commits were squashed

There is no easy way to make these changes, and I think it makes sense to do all the work in one PR instead of many repetitive and painful PRs. I suggest that the embedded FIXME and TODO comments remain, that the PR be merged, and then follow-on PRs be created to address related FIXME and TODO issues.

Time to see if Travis builds this thing

Time to see if Travis builds this thing RBMHTechnology#2

Forgot this file

Moved and renamed

Java serialization started but not complete

Scala serialization works for ActorExample

Runs but with errors shown as comments

Scala serialization works for CommunicationExample

Removed redundant message classes

Scala serialization works for ConditionalExample

Made RichVectorTime, which serializes and deserializes easily

Made RichVectorTime, which serializes and deserializes easily

Made RichVectorTime, which serializes and deserializes easily

Made RichVersioned, which serializes and deserializes easily

Deleted redundant message types

Hived serialization example from user-guide

Added list of serialization packages

bit by bit

Sphinx output looks good
@krasserm
Copy link
Contributor

Thank you for your work on re-structuring the examples. I appreciate your contributions but the pull request is by far too large. IMO it can easily be broken into several smaller ones, for example:

  • re-organize the existing examples under a single root. The single root shall remain an Eventuate sub-module not a completely isolated sbt project. If you really want to have a completely isolated example (demonstrating all the necessary sbt and dependency settings) move it to a separate repository.

  • change the examples to become executable from the sbt command line without affecting the content in the documentation. The only required change in the documentation might be a hint where the runnable examples are located and how to run them.

  • revision the user guide and other parts of the documentation where you think wording or content can be improved.

  • ...

This is only a suggestion how work can be split into several smaller pull requests. I’m happy to discuss alternatives. Also, after a quick walk through your changes, I have some general notes (in random order):

  • The organization is the example sbt files is com.micronautics. This should be com.rbmhtechnology.
  • There are several .travis.yml files in the examples. I don’t see that they are necessary.
  • The build.sbt files are highly redundant/repetitive. Try to avoid that, otherwise, it’s going to be a maintainence hell when dependencies and/or general settings change.
  • Please do not just comment out lines in build.sbt or application.conf. Remove them if they are not needed.
  • Please use meaningful commit messages. The current one (when expanded) is confusing.
  • Most of the changes in spark.rst are new linebreaks. Either follow that style in all .rst files or none of them. I’d prefer the latter (i.e. the current state).
  • There are some .pyc file checked in. Please remove them.
  • Do not use TODO or FIXME in files to track work to be done in the future. Use the issue tracker instead.
  • Do you insist on having ... and Mike Slinn in the copyright headers? I don’t know if this is in conflict with the CLA. Have to check that /cc @purthaler.

I'll give more detailed feedback to the smaller pull requests.

@mslinn
Copy link
Contributor Author

mslinn commented Mar 31, 2017 via email

@krasserm
Copy link
Contributor

krasserm commented Apr 4, 2017

It is important that the examples use the very same settings as Eventuate itself for compilation and running. Otherwise the quality of the examples cannot be guaranteed. Requiring discipline from developers to maintain that at several locations has proven to be error-prone in the past, that's why I don't want to violate DRY in the Eventuate build.

If you want to demonstrate how to setup a build.sbt file from scratch, either create a separate example repository or provide a template for Lightbend Activator, Giter8 or whatever. For example, @volkerstampa created a very useful activator template for the Eventuate example application.

It would be great to have several templates for different use cases e.g. single location app, multi-location app, CmRDT app, ... from which you can create a running project from the command line. Ideally, the created project contains sample code with running unit and integration tests.

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.

2 participants