Skip to content

Created Color-Me module#62

Merged
SCDerox merged 4 commits intoScootKit:mainfrom
hfgd123:main
May 25, 2022
Merged

Created Color-Me module#62
SCDerox merged 4 commits intoScootKit:mainfrom
hfgd123:main

Conversation

@hfgd123
Copy link
Copy Markdown
Contributor

@hfgd123 hfgd123 commented May 19, 2022

Created Color-Me module. Requesting review to check for mergablilty

@hfgd123
Copy link
Copy Markdown
Contributor Author

hfgd123 commented May 20, 2022

Requesting review by @SCDerox

Copy link
Copy Markdown
Member

@SCDerox SCDerox left a comment

Choose a reason for hiding this comment

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

Looking pretty good!
While I haven't done my usual code-quality check (if you want me to do that, please message me), there are a few issues I encountered when using this module:

  • Position of the new role is always at the bottom. Staff should decide below which role the new role should be created (message me if you need help with that)

  • Show the user how long the need to wait until their cooldown expires (use dateToDiscordTimestamp(role.timestamp, 'R' from helpers.js)

  • Entering something random into the color field will show an error about the max-role count, which is not optimal as the input is invalid rather then the role-count is maxed-out

@SCDerox

This comment was marked as off-topic.

@hfgd123
Copy link
Copy Markdown
Contributor Author

hfgd123 commented May 22, 2022

Show the user how long the need to wait until their cooldown expires (use dateToDiscordTimestamp(role.timestamp, 'R' from helpers.js)

wouldn't this just tell the user, when they edited their role the last time? How can I do math things (english 10/10 :hAA:) with dates?

@SCDerox

This comment was marked as off-topic.

@SCDerox
Copy link
Copy Markdown
Member

SCDerox commented May 22, 2022

I believe dateToDiscordTimestamp(new Date(role.timestamp.getTime() + moduleConf['updateCooldown'] * 3600000), 'R') should work

@hfgd123 hfgd123 requested a review from SCDerox May 24, 2022 18:41
Copy link
Copy Markdown
Member

@SCDerox SCDerox left a comment

Choose a reason for hiding this comment

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

Everything works for me, thanks for your contribution!

@SCDerox SCDerox merged commit 056b1b9 into ScootKit:main May 25, 2022
@hfgd123 hfgd123 deleted the main branch May 25, 2022 20:14
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