Skip to content

Week 7: Project GitHub Tracker#38

Open
dandeloid wants to merge 10 commits intoTechnigo:mainfrom
dandeloid:main
Open

Week 7: Project GitHub Tracker#38
dandeloid wants to merge 10 commits intoTechnigo:mainfrom
dandeloid:main

Conversation

@dandeloid
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@hemmahosjessi hemmahosjessi left a comment

Choose a reason for hiding this comment

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

Hopefully it gets easier to make these code reviews along the Bootcamp. It would be nice to give good feedback that you actually can use 😅. Really nice job! Only thing was in the CSS that I would like some separation between the blocks of code. Keep up the good work! I find it really inspiring hearing you talk about the code we are doing. You have a curiosity that gives me the courage to experiment! Thank you!

@@ -1,4 +1,27 @@
//DOM-selector for the canvas 👇
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey Daniel, a bit hard to review Javascript when struggling with the understanding. But I know that you have that 😄. You have a good structure in the code, easy to follow with the indents and so on.

document.getElementById(`comment1-${repo.name}`).innerHTML += `${commitDate1}: "${data[0].commit.message}"`
}
if (data[1].author.login === 'dandeloid'){
const commitDate2 = new Date (data[1].commit.committer.date).toDateString()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe this part could have an indentation to easily see that it is inside the if? 😄

--gblue: rgb(87, 166, 255);
}
body {
background: #FFECE9;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In the css I think it would be nice with an empty line between. Easier to read. It gets a bit hard to seperate it now.

width: 30px;
margin-right: 10px;
}
.bio,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah, so this is the way to have two classes with the same styling! ⭐️

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