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.

Sign up for GitHub

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

Jump to bottom

Home page news #76

Merged
merged 8 commits into from
Dec 14, 2023
Merged

Home page news #76

merged 8 commits into from
Dec 14, 2023

Conversation

suejinkim20
Copy link
Collaborator

No description provided.

@suejinkim20 suejinkim20 requested review from Woozl and mbwatson December 6, 2023 15:37
Woozl
Woozl approved these changes Dec 6, 2023
View reviewed changes
Copy link
Member

@Woozl Woozl left a comment

Choose a reason for hiding this comment

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

Looks good, I don't think it needs to be in a useEffect though, we should probably just stick that code in the getServerSideProps for now, although you'll have to use Promise.all/Promise.allSettled to fetch the projects + articles in parallel.

pages/index.js Outdated
Comment on lines 13 to 14
const [loading, setLoading] = useState(true);
const [error, setError] = useState(null);
Copy link
Member

Choose a reason for hiding this comment

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

These are unused

@suejinkim20 suejinkim20 requested a review from Woozl December 13, 2023 15:01
Woozl
Woozl approved these changes Dec 14, 2023
View reviewed changes
Copy link
Member

@Woozl Woozl left a comment

Choose a reason for hiding this comment

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

👍

@suejinkim20 suejinkim20 merged commit 367856c into develop Dec 14, 2023
@suejinkim20 suejinkim20 deleted the home-page-news branch June 6, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Woozl Woozl Woozl approved these changes

@mbwatson mbwatson Awaiting requested review from mbwatson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@suejinkim20 @Woozl