Skip to content

GitHub Tracker by Joanna Lodell - Week 7#97

Open
joannalodell19 wants to merge 11 commits intoTechnigo:mainfrom
joannalodell19:main
Open

GitHub Tracker by Joanna Lodell - Week 7#97
joannalodell19 wants to merge 11 commits intoTechnigo:mainfrom
joannalodell19:main

Conversation

@joannalodell19
Copy link
Copy Markdown

Copy link
Copy Markdown

@JensBergqvist JensBergqvist left a comment

Choose a reason for hiding this comment

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

Hi Joanna! You made a beautiful and well working site! You should be really proud of yourself!
I only saw some minor things, like console.logs still there in the JS on row 44, 47, 50 and 53. Like i wrote in the comment I forget to remove these aswell all the time. But I think we're gonna get asked by Technigo to go back and remove them for any new projects now.
Keep up the awesome work!

Comment thread code/script.js
fetch(API_PROFILE)
.then(res => res.json())
.then(data => {
// console.log(data)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To make the code more clean, try and remember to remove console.logs before sending it. I miss these all the time aswell so I'm not critizicing in any way. I belive they will be much harder on us removing these further on.

Comment thread code/script.js

getRepos()

// Pull requests for each project
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good comment! Makes it easy to understand what the code does, even for someone who is new to coding and maybe dosen't understand exactly what the function does!

Comment thread code/script.js
Comment on lines +82 to +87
// If pull request done by user, getCommits function is invoked
if (myPullRequest) {
getCommits(myPullRequest.commits_url, repo.name);
} else {
document.getElementById(`commit-${repo.name}`).innerHTML =
"No pull request done by user";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Very nice code! I struggled alot to make my if statements actually work, and my code became way longer to get the same result!

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.

2 participants