Skip to content

Examples: update led syntax #1598

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

Merged
merged 3 commits into from
Aug 17, 2019

Conversation

ryanjgill
Copy link
Contributor

No description provided.

@dtex
Copy link
Collaborator

dtex commented Aug 17, 2019

Cool! A couple of comments based on the esc examples Rick created last week:

Use destructuring on the J5 import to grab just the classes you need rather than grabbing the parent five object.

Let the board ready handler be bound to board. It is the most likely target for this throughout a J5 program (the REPL, accessing the io object, etc ). In other words, no fat arrow on board.on("ready", function() { ... });.

I was about to say something about grunt examples but I see you just updated your PR so I'm guessing you saw the error.

const {Board, LED} = require("../lib/johnny-five.js");
const board = new Board();

board.on("ready", function() {
  const led = new Led(13);

  // This will grant access to the led instance
  // from within the REPL that's created when
  // running this program.
  this.repl.inject({
    led
  });

  led.blink();
});

@dtex
Copy link
Collaborator

dtex commented Aug 17, 2019

LGTM!

@dtex dtex merged commit 507c5c0 into rwaldron:master Aug 17, 2019
@rwaldron
Copy link
Owner

Let the board ready handler be bound to board. It is the most likely target for this throughout a J5 program (the REPL, accessing the io object, etc ). In other words, no fat arrow on board.on("ready", function() { ... });.

If someone wants to do the work of replacing all the occurrences of this with board (where appropriate), we could move the examples to use an arrow in the "ready" handler.

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.

3 participants