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

Instructor-training assignment #245

Closed
wants to merge 3 commits into from
Closed

Conversation

jordwil
Copy link

@jordwil jordwil commented Feb 16, 2017

Added an additional feature to the challenge, mainly to introduce reserved words as well as add descriptions to the solutions.

Added an additional feature to the challenge, mainly to introduce reserved words as well as add descriptions to the solutions.
Copy link
Member

@naupaka naupaka left a comment

Choose a reason for hiding this comment

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

Because the explanations take up more than a single line, you might consider making them into an unordered list to increase legibility. You might also want to have the variable names displayed in monospaced code style.

> > _age - We can only start variable names with letters or dots not followed by a number.
> > min-length - This contains a dash "-" character that can't be used when naming a variable. We can only use letters,
> > numbers, dots, and underline "_" characters.
> > 2widths - 2widths - We can't start a variable name with a number.
Copy link
Member

Choose a reason for hiding this comment

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

Why is '2widths' repeated?

> > min-length - This contains a dash "-" character that can't be used when naming a variable. We can only use letters,
> > numbers, dots, and underline "_" characters.
> > 2widths - 2widths - We can't start a variable name with a number.
> > if - If is a special case called a reserved word. We use it for control structures that will be discussed at a later
Copy link
Member

Choose a reason for hiding this comment

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

I might say something like: "If is a special type of word called a reserved word. There are about 20 of these reserved words, and they represent things that cannot be directly overwritten during normal use of the assignment operator because they are a core part of the language itself.

@naupaka
Copy link
Member

naupaka commented Feb 16, 2017

You should also give the PR a descriptive name that says what it does to the lesson. Then in the description text you can note that it is being submitted as part of instructor training.

@naupaka
Copy link
Member

naupaka commented Feb 17, 2017

Hi @jordwil thanks for the modifications. I didn't notice this the first time, but your PR contains modifications to the .md file for the lesson and not the .Rmd. Since we generate the .md files from the .Rmd files, the plain markdown will get overwritten whenever we rebuild the site content. The guide for contributions to lessons is here. It will be cleaner and potentially easier if we close this PR and you reopen a new one with just modifications to the corresponding .Rmd file in the _episodes_rmd directory. Let me know if that sounds reasonable.

@naupaka
Copy link
Member

naupaka commented Apr 29, 2017

Hi @jordwil - do you think you'll have a chance to migrate these changes over to the Rmd instead of the md files? Otherwise I'll close this PR since there hasn't been much activity for a few months.

@jordwil
Copy link
Author

jordwil commented May 5, 2017 via email

@jordwil jordwil closed this May 6, 2017
@jordwil jordwil deleted the patch-1 branch May 6, 2017 00:03
rgaiacs pushed a commit to rgaiacs/swc-r-novice-gapminder that referenced this pull request May 6, 2017
rgaiacs pushed a commit to rgaiacs/swc-r-novice-gapminder that referenced this pull request May 6, 2017
Moving overview of existing lesson materials to Carpentries lesson.
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