Skip to content
New issue

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

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

Already on GitHub? # to your account

preoop task #91

Merged
merged 3 commits into from
Aug 16, 2022
Merged

preoop task #91

merged 3 commits into from
Aug 16, 2022

Conversation

AsaMitaka
Copy link
Contributor

Building a Tiny JS World (pre-OOP)

Demo
Code

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

@yaripey
Copy link

yaripey commented Aug 15, 2022

Hi! Good job, could you please remove the index.html file since the work was mainly in the .js one?

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

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

@AsaMitaka good job!

Check the comment below and also add semicolons where appropriate. Adhering to a consistent code style is important.


const inhabitants = [dog, cat, man, woman, catWoman];

function aboutObj(obj) {
Copy link
Member

Choose a reason for hiding this comment

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

By convention, function names should start with a verb. This way we know it does something rather than contains/represents something.

@OleksiyRudenko OleksiyRudenko self-assigned this Aug 16, 2022
Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

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

@AsaMitaka well done! Approved.

In your future PRs, when you implemented all required changes either click Re-request review or, if it is not available, comment on a PR that you accomplished all required fixes and mention the reviewer.

image

@OleksiyRudenko OleksiyRudenko merged commit 6f6e409 into kottans:main Aug 16, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants