Skip to content

Comments

Tim matt#2

Open
matstap wants to merge 11 commits intocodefellows-seattle-301d21:masterfrom
matstap:tim-matt
Open

Tim matt#2
matstap wants to merge 11 commits intocodefellows-seattle-301d21:masterfrom
matstap:tim-matt

Conversation

@matstap
Copy link

@matstap matstap commented Jun 26, 2017

Single-line Summary

Today, Tim and Matthew paired together. It took about 3 hoours

Reflect and summarize on your process for each TODO item :

  1. First, we wrapped article.js in an IIFE
  2. Next, we went through article.js and used map/reduce chains to transform the data
  3. Finally, we completed the fetchAll function and called it in index.html and admin.html

Checklist (before submitting, fill in each set of square brackets with an 'x')

  • We have titled the Pull Request similar to our branch name (ex: 'brian-rick').
  • This PR includes commits from both myself and my partner; e.g. We followed good pair programming practices by switching driver/navigator roles.
  • There is no extraneous, unrelated code included in this PR.
  • We have summarized our TODO: process above.

// of functions. So if we set a variable equal to the result of a .map, it will be our transformed array.
// There is no need to push to anything.

rows.map(ele => Article.all.push(new Article(ele)));

Choose a reason for hiding this comment

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

This is improper use of .map(). You do not need to use .push() to fill Article.all since .map() returns a new array.

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