Skip to content
This repository has been archived by the owner on Jun 12, 2020. It is now read-only.

Feat anniversaire #11

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open

Conversation

Westixy
Copy link
Member

@Westixy Westixy commented Aug 4, 2018

Implémentation à discuter du plugin anniversaire liée a l'issue #9

  • ajout de la dépendance node-cron

Copy link
Member

@Xstoudi Xstoudi left a comment

Choose a reason for hiding this comment

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

Quelques corrections et réponse à apporter, merci pour l'implémentation Westixy

.array().forEach(m => {
const age = (new Date()).getUTCFullYear() - m.joinedAt.getUTCFullYear()
if (m.id === guild.ownerID) {
guild.channels.find('name', 'annonces').send(`La communauté fête ses ${age} ans !`)
Copy link
Member

Choose a reason for hiding this comment

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

@fraxken n'aura donc jamais la chance de se faire souhaiter son anniversaire ahah !

Copy link
Member Author

Choose a reason for hiding this comment

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

si si

Copy link
Member

Choose a reason for hiding this comment

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

En effet

guild.channels.find('name', 'annonces').send(`La communauté fête ses ${age} ans !`)
}
if (age === 1) m.send(':birthday: Déjà 1 an que tu es sur la communauté ! :vulcan:')
m.send(`:birthday: La viellesse arrive, ça fait ${age} ans que tu es arrivé sur l'ESCommunity :vulcan:`)
Copy link
Member

Choose a reason for hiding this comment

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

Là il faudrait unelsesinon ça va partir sur une double-annonce à chaque première bougie.

* @param {Guild} guild Server where to apply the plugin
*/
static init (guild) {
cron.schedule('0 0 10 * * *', () => {
Copy link
Member

Choose a reason for hiding this comment

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

  • Ton cron doit pas être valide : https://crontab.guru/#0_0_10___*
  • Quel intérêt d'ajouter une dépendance plutôt que de faire un setInterval ?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 @Xstoudi pour les dépendances

Copy link
Member Author

Choose a reason for hiding this comment

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

  • il est valide c'est juste une implémentation de cron qui autorise les secondes en premier (https://www.npmjs.com/package/node-cron)
  • par pur soucis de lisibilité (et la dépendance est tellement light que j'ai trouvé intéressant)

Copy link
Member

Choose a reason for hiding this comment

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

  • Yes bien vu.
  • Après réflexion c'est pas bête, ça permet aussi d'avoir le truc exécuté toujours à la même heure.

* @param {Guild} guild Server where to apply the plugin
*/
static init (guild) {
cron.schedule('0 0 10 * * *', () => {
Copy link
Member

Choose a reason for hiding this comment

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

  • Yes bien vu.
  • Après réflexion c'est pas bête, ça permet aussi d'avoir le truc exécuté toujours à la même heure.

@fraxken
Copy link
Member

fraxken commented Sep 26, 2018

@Xstoudi @Westixy La fonctionnalité est t'elle bonne pour un merge ?

@Westixy
Copy link
Member Author

Westixy commented Sep 26, 2018

Il me semble oui si les messages vous conviennent (je les avais mis comme ca sans trop réfléchir :) )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants