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

Recent commits page #118

Merged
merged 6 commits into from
Apr 26, 2024
Merged

Recent commits page #118

merged 6 commits into from
Apr 26, 2024

Conversation

SilasPeters
Copy link
Member

This feature adds a new 'recent commits' page, which displays the last X commits. These commits are fetched from all repositories within svsticky. This aims to act as a 'thank you' for the developers, visible to all members of Sticky! It also sort of promotes joining us.

- Fix font size and alignment
- Sort by date
- Use GH Api token
@TobiasDeBruijn
Copy link
Member

Wat vindt men?:
image

@SilasPeters
Copy link
Member Author

Goed! Ik zou alleen ook nog graag de tijd willen zien, naast de date.

Ook vind ik het een interessante aanpak om de GH API key te gebruiken, en alleen commits te laten zien van een array van repos. Fijn om zo wat controle te hebben. Wel denk ik dat die lijst in de .env moet zitten. Dan is deze lijst makkelijker te vinden, maar ook is dan de code flexibeler voor andere verenigingen (amino bijvoorbeeld, gebruikt ook de Radio).

Voordat ik een volledige nitpick review op de code ga doen, (heb het meeste hier al verteld though), vraag ik mij af of @lukeksnceuoibceiofegofiqw al wijzigingen open had staan die hier mee botsen, aangezien hij hier ook mee bezig was. Wat vind jij Luke?

@SilasPeters
Copy link
Member Author

Bovendien heb ik binnenkort een nieuwe officiële poster voor de CommITCrowd. Zou je dus de poster kunnen verplaatsen naar Contentful?

…commits page if there are no repositories configured
@TobiasDeBruijn
Copy link
Member

De API key was nodig ivm ratelimits, die zijn enorm laag zonder auth.

@TobiasDeBruijn
Copy link
Member

@SilasPeters @lukeksnceuoibceiofegofiqw Nog meningen? Zou tof zijn als we 'm voor de open dag live kunnen hebben (lijkt mij)

@SilasPeters
Copy link
Member Author

@SilasPeters @lukeksnceuoibceiofegofiqw Nog meningen? Zou tof zijn als we 'm voor de open dag live kunnen hebben (lijkt mij)

Aangezien luuk nog niet gereageerd heeft, zeg ik gooi maar open voor review!

@SilasPeters SilasPeters marked this pull request as ready for review April 24, 2024 16:48
Copy link
Member Author

@SilasPeters SilasPeters left a comment

Choose a reason for hiding this comment

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

Since I am the author of this PR I cannot request changes, but here we go haha

sample.env Outdated Show resolved Hide resolved
src/App.jsx Outdated Show resolved Hide resolved
src/App.jsx Outdated Show resolved Hide resolved
src/components/Commits.jsx Outdated Show resolved Hide resolved
src/helpers/env.js Outdated Show resolved Hide resolved
@TobiasDeBruijn
Copy link
Member

One final look @SilasPeters ?

@SilasPeters SilasPeters merged commit 281997c into master Apr 26, 2024
@SilasPeters SilasPeters deleted the feat/recent-commits branch April 26, 2024 10:47
@SilasPeters
Copy link
Member Author

Thanks @TobiasDeBruijn !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Proposes (changes to) a feature good first issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants