Skip to content

Naushin Malik-Project github Tracker#55

Open
nama0027 wants to merge 9 commits into
Technigo:mainfrom
nama0027:main
Open

Naushin Malik-Project github Tracker#55
nama0027 wants to merge 9 commits into
Technigo:mainfrom
nama0027:main

Conversation

@nama0027

@nama0027 nama0027 commented Oct 4, 2021

Copy link
Copy Markdown

The challenge for this week was the limit of 60 request per hour for github API. Therefore, first, I worked on functionality without any styling, then, I did styling with placeholders and at the end i merged both of them to have the final version.

I have completed most of the requirements. If i had more time, I would have add sorting and more visual representation of data.

Live at: https://naushin-github.netlify.app/

@MT-dotse MT-dotse left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job Naushin,

The website works nicely and even though I haven’t myself made a search function or fetched commit comments it is easy to follow your javascript code. I am very impressed by your search function, well done!

The site is responsive and works on various displays..

You have met the blue, most in the red and the black requirement.

Overall, a great job on the project and I look forward to working together and learn some more javascript from you! :)

Comment thread code/script.js
.then(data => {
console.log(data);
const userPullReq = data.find((pull) =>
(repo.owner.login === pull.user.login || pull.title.includes('Naushin'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That is clever to use the include method to fetch your pullrequest!

Comment thread code/script.js



function getCommits (commitUrl, repoName) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Even if I haven't fetched commits for my own project, I find you code clean and easy to follow. Good job!

Comment thread code/script.js
.then(res => res.json())
.then(data => {
if (data.length!== 0){
console.log('review', 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.

It is great that you have used a lot of console.logs, but remember to remove them when it is time for launch.

Comment thread code/style.css
font-size: 14px;
line-height: 24px;
vertical-align: middle;
border: 1px solid var(--secondary-color );

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That is really clever to structure the colors using primary and secondary.

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