Skip to content

Conversation

@EliaYazdi
Copy link
Owner

No description provided.

Copy link

@mattiaslundberg mattiaslundberg left a comment

Choose a reason for hiding this comment

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

Added some pointers!

Don't forget the step to move to classes later.

window.onload = () => main(HYF_REPOS_URL);
}
jsonData.forEach(contributor => {
createAndAppend('div', contribs, { text: contributor.login, class: 'contributor' })

Choose a reason for hiding this comment

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

contribs here isn't what you expect. It's the promise from line 63 and not the dom-element from line 35, you need to get it from the dom again (like you do on line 35) and use it in this function.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I changed this. but now the contrinutors_url is undefined!

Choose a reason for hiding this comment

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

Not related! Have a look at how you call getContributorInformation and what arguments it requires

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think I get your point. I forgot to put the argument 'data' in the getContributorInformation function.
I should figure out what to put there !

Copy link

@mattiaslundberg mattiaslundberg left a comment

Choose a reason for hiding this comment

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

The async/await rewrite looks good! You also have to do the rewrite with classes to pass week 3, let me know if you need any assistance!

Close

Copy link

@mattiaslundberg mattiaslundberg left a comment

Choose a reason for hiding this comment

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

Approved

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