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

all issues fixed #109

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

zusko33
Copy link

@zusko33 zusko33 commented Sep 20, 2023

No description provided.

Copy link
Member

Choose a reason for hiding this comment

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

Great job adding the missing newline at the end of the file. It's a small detail, but it's good practice to include it.

On line 7, it looks like you changed the field name from 'company_name' to 'name'. It's important to make sure that this change is reflected consistently throughout the codebase.

Other than that, the changes look good!

Copy link
Member

Choose a reason for hiding this comment

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

Great job on the changes! The code looks much cleaner and more readable now. I have a few suggestions:

  1. In lines 3 and 4, it seems like the indentation is inconsistent. It would be better to use either spaces or tabs consistently throughout the file.

  2. In lines 15, 19, 23, and 27, there are unnecessary comments with extra spaces at the beginning. You can remove those extra spaces to keep the comments consistent.

  3. In lines 32-34, you can use a comma to separate the 'th' and 'td' selectors instead of a new line to improve readability.

  4. It's a good practice to have a newline at the end of the file. You can add a newline character at the end of the file to follow this convention.

Overall, great job on the changes! Keep up the good work.

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