anguillanneuf commented on pull request #14962: URL: https://github.com/apache/beam/pull/14962#issuecomment-863579457
Overall looks great. Gardening project, moon phases, the notebook has got personality and I like it a lot :) Some comments as I was going through the [notebook](https://colab.research.google.com/github/davidcavazos/beam/blob/tour-of-beam/examples/notebooks/tour-of-beam/windowing.ipynb): 1. `s/lets/let's` 2. Suggested edit - redundancy: "In our example, the "processing" is done by PrintElementInfo which simply prints the element with its window information. ~For windows of three months every month~, each element is processed three times, one time per window." 3. Suggested edit - use a bulleted list: "Sliding windows allow us to do just that. We need to specify the window size in seconds just like with FixedWindows. We also need to specify a window period in seconds, which is how often we want to emit each window." 4. Suggested edit - redundancy - future tense: "If the next event happens within the next 30 days ~or less, like 20 days after the previous event,~ the session window [will extend and cover..] extends and covers that as well. If there are no new events for the next 30 days, the session window [will close..] closes and is emitted." 5. I had to scroll up to remember what input events look like in the later examples. It would be nice to include a screenshot of the input events to the right of the nice bar charts, then these screenshots can be easily used by other tutorials/talks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
