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

chitter-challenge #2086

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

chitter-challenge #2086

wants to merge 5 commits into from

Conversation

yulingyou
Copy link

No description provided.

@cbai123
Copy link

cbai123 commented Dec 5, 2022

In the repository classes the find and all methods for posts and users are quite long, perhaps part of it could be put into a private method to reduce the size of method.

@cbai123
Copy link

cbai123 commented Dec 5, 2022

All tests passed except for 2 in post_repository_spec, it expected create_at to equal a timezone of +08 but was receiving +01. I think this problem comes in during seeding the tables, the date put in has no timezone specified, so maybe in my computer it’s defaulting to +1.

@cbai123
Copy link

cbai123 commented Dec 5, 2022

Website works when using rackup.

Good use of checking for invalid input.

@cbai123
Copy link

cbai123 commented Dec 5, 2022

Could maybe add tests in app_spec that check when a new post is added it can be found on the home page.

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