Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tutorial with exercises #6

Open
wants to merge 17 commits into
base: exercises
Choose a base branch
from
Open

Conversation

Crista2019
Copy link
Contributor

Description

Set out instructions in ui/README and server/README with tasks that focus on teaching basic Swim features through hands-on implementation such as:

  • adding multiple web agents
  • sending data from histogram to new value lane(s)
  • adjusting the UI to reflect server-side changes

Added TODO comments with hints in respective files to guide user where/how to follow these instructions

Points to be addressed before merge

  • Any style conventions
  • Suggested refactoring/code clarification
  • Missing aspects of the code that would be useful for a user

@Crista2019 Crista2019 mentioned this pull request Jul 14, 2020
4 tasks

// HINT: Use the valueLane() method to instantiate the lane
// HINT: Use the .didSet() lifecycle callback to log a message showing updates to stats

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding some hints for intermediary statistic lanes here-ish would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully the following comments (lines 23-30) better explain one's options for how to implement lanes. (Let me know if the wording can be made clearer.)


// TODO: update stats with update logic

// HINT: access new data sent to histogram with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's replace this with n.get("count").longValue() for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if the comments from (41-48) get too overwhelming. If so, I can remove the #RECOMMENDED and #ALTERNATIVELY sections and just leave it at suggesting n.get("count).longValue()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like the RECOMMENDED and ALTERNATIVELY!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants