Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project-github-tracker #119

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Project-github-tracker #119

wants to merge 17 commits into from

Conversation

Mimmisen
Copy link

@Mimmisen Mimmisen commented Feb 28, 2022

code/script.js Outdated
<img src="${data.avatar_url}" alt="Avatar of ${data.login}">
</a>
</div>
<a href="https://github.com/Mimmisen"><h1 class="username">${data.name}<h1></a>
Copy link

Choose a reason for hiding this comment

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

data.name gives you your actual name not the username (Mimmisen), you could try to fix that and try out how you get your username (that was also in minimum requirements I think)

Copy link
Author

Choose a reason for hiding this comment

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

Yes I just wanted to display my name instead with the link to my github but that's true that it says that it should say your username in the requirements I see now so I have changed it

</div>
</div>
`;
});
Copy link

Choose a reason for hiding this comment

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

here you are missing the names of the main branches (also minimum requirements), so you could add one line more to your innerHTML (if I remember correctly it was repo.default_branch how you get the names of the main branch)

Copy link
Author

Choose a reason for hiding this comment

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

Yes and now I have added it!

Copy link

@kolkri kolkri left a comment

Choose a reason for hiding this comment

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

Great job! Your code is very clean and well structured, which make it easy to read.

Your page fulfills all the minimum requirements (see below) except two: 1) you are printing your real name instead of username, and 2) your are missing the names of the main branches.

Here is a list of the minimum requirements:

  1. A list of all repos that are forked from Technigo Your username and profile picture
  • Most recent update (push) for each repo
  • Name of your default branch for each repo
  • URL to the actual GitHub repo
  • Number of commits for each repo
  • It should be responsive (mobile first)
  • A visualisation, for example through a pie chart, of how many projects you've done so far, compared to how many you will do (in total it will be 19 weekly projects 🥳) using [Chart.js](https://www.chartjs.org/).

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