Skip to content
This repository was archived by the owner on May 14, 2024. It is now read-only.

Conversation

@amirno1
Copy link

@amirno1 amirno1 commented Jun 13, 2018

I know my code looks like a chaos but everything works perfectly. I started in this way and i had nothing to do but continue. I really appriciate it if you can tell me how and where in my code, could i make it easier .Last thing to say ,the css part is really simple because it has been done in no time. thanks for your feedback in advance.

Copy link

@harditsingh harditsingh left a comment

Choose a reason for hiding this comment

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

You did a great job on the assignment and the code looks pretty good! I have put some comments though, please go through them once. Good luck!

function renderContributorsInformationToDom() {

let hyfRepositoryAtTheMoment = repositoryNameLink.innerText;
const gitHubUrl = 'https://api.github.com/repos/HackYourFuture/' + hyfRepositoryAtTheMoment + '/contributors';

Choose a reason for hiding this comment

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

You should not put the URL inside functions and in multiple places, it can get confusing really quickly

const hyfRepositoriesRequest = new XMLHttpRequest();
hyfRepositoriesRequest.open('GET', gitHubUrl, true);
hyfRepositoriesRequest.send();
hyfRepositoriesRequest.onreadystatechange = repositoriesRequest;

Choose a reason for hiding this comment

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

You have not handled any errors if server is not available. This way the user doesn't know why the website is not working when a server is not available.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants