Add Birthday Announcement Feature #29

2023-11-17T05:00:42-06:00

JMTNTBANG commented

2023-11-17 05:00:42 -06:00

(Migrated from github.com)

Anyone who wants to look over my code feel free to, let me know ill add you as a reviewer

MaddyGuthridge (Migrated from github.com) reviewed

2023-11-17 05:00:42 -06:00

BobVonBob (Migrated from github.com) reviewed

2023-11-17 05:00:42 -06:00

MaddyGuthridge (Migrated from github.com) requested changes

2023-11-17 05:46:54 -06:00

MaddyGuthridge (Migrated from github.com) left a comment

Looks like a good start, but could benefit from a bit of tidying up and refactoring

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

Why 14 and 0? These particular values aren't clear - perhaps add a comment explaining why they are used? My best guess is that you only want to say "happy birthday" to users once per day, but if so, why is this function called so often (guessing every minute)?

Why `14` and `0`? These particular values aren't clear - perhaps add a comment explaining why they are used? My best guess is that you only want to say "happy birthday" to users once per day, but if so, why is this function called so often (guessing every minute)?

Use boolean operations to simplify your code and reduce indentation

                            if channel.topic is not None and "YouTube Ping" in channel.topic:

Use boolean operations to simplify your code and reduce indentation ```suggestion if channel.topic is not None and "YouTube Ping" in channel.topic: ```

Overall, this code is far too indented for my liking. It could be worthwhile setting up some CI (using something like flake8 to lint the code) to automatically detect these kinds of issues

Overall, this code is far too indented for my liking. It could be worthwhile setting up some CI (using something like `flake8` to lint the code) to automatically detect these kinds of issues

aussie_clock.py

					
				@ -117,0 +128,4 @@

				                        if b_day.year > 1:

				                            await channel.send(f'Merry {today.year - b_day.year}th Birthmas {user.mention}!')

				                        else:

				                            await channel.send(f'Merry Birthmas {user.mention}!')

Add a docstring to ensure that the behaviour of the code is clear

aussie_clock.py

					
				@ -117,0 +129,4 @@

				                            await channel.send(f'Merry {today.year - b_day.year}th Birthmas {user.mention}!')

				                        else:

				                            await channel.send(f'Merry Birthmas {user.mention}!')

Consider using the aiofiles library to read this data asynchronously

Consider using the [`aiofiles` library](https://pypi.org/project/aiofiles/) to read this data asynchronously

birthdays.py

This code seems unfinished - I'd expect something inside on_ready

This code seems unfinished - I'd expect something inside `on_ready`

commands/birthday.py

					
				@ -0,0 +1,61 @@

				import bot

Use a with statement to avoid the need to manually update.close() the file

        with open("birthdays.json", "w") as update:
            update.write(json.dumps(birthdays, indent=3))

Use a `with` statement to avoid the need to manually `update.close()` the file ```suggestion with open("birthdays.json", "w") as update: update.write(json.dumps(birthdays, indent=3)) ```

BobVonBob (Migrated from github.com) reviewed

2023-11-17 06:20:50 -06:00

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

You can get rid of three levels of nesting by using the .date() method, combining conditions, and taking advantage of short circuit evaluation for the None check.

        if today.date() == b_day.date() and today.hour == 14 and today.minute == 0:
            for guild in client.guilds:
                for channel in guild.text_channels:
                    if channel.topic is not None and "YouTube Ping" in channel.topic:

You can get rid of three levels of nesting by using the `.date()` method, combining conditions, and taking advantage of short circuit evaluation for the None check. ```suggestion if today.date() == b_day.date() and today.hour == 14 and today.minute == 0: for guild in client.guilds: for channel in guild.text_channels: if channel.topic is not None and "YouTube Ping" in channel.topic: ```

Wait no the year should be discarded. In that case something like one of the two following would work.

Tuple comparison:

        if (today.month, today.day, today.hour, today.minute) == (b_day.month, b_day.day, 14, 0):
            for guild in client.guilds:
                for channel in guild.text_channels:
                    if channel.topic is not None and "YouTube Ping" in channel.topic:

Just combining the month and day:

        if today.month == b_day.month and today.day == b_day.day:
            if today.hour == 14 and today.minute == 0:
                for guild in client.guilds:
                    for channel in guild.text_channels:
                        if channel.topic is not None and "YouTube Ping" in channel.topic:

Wait no the year should be discarded. In that case something like one of the two following would work. Tuple comparison: ```suggestion if (today.month, today.day, today.hour, today.minute) == (b_day.month, b_day.day, 14, 0): for guild in client.guilds: for channel in guild.text_channels: if channel.topic is not None and "YouTube Ping" in channel.topic: ``` Just combining the month and day: ```suggestion if today.month == b_day.month and today.day == b_day.day: if today.hour == 14 and today.minute == 0: for guild in client.guilds: for channel in guild.text_channels: if channel.topic is not None and "YouTube Ping" in channel.topic: ```

commands/birthday.py

					
				@ -0,0 +1,61 @@

				import bot

Frank deserves a capital F.

Here too.

Ditto.

And longer and get shouldn't be capitalized.

        description='No longer get birthday celebrations from Frank'

And longer and get shouldn't be capitalized. ```suggestion description='No longer get birthday celebrations from Frank' ```

        description='Get birthday celebrations from Frank'

```suggestion description='Get birthday celebrations from Frank' ```

        description='Get birthday celebrations from Frank'

```suggestion description='Get birthday celebrations from Frank' ```

BobVonBob (Migrated from github.com) reviewed

2023-11-17 06:28:50 -06:00

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

Better still only calling it once a day like Miguel suggested would remove the hour and minute check altogether.

JMTNTBANG (Migrated from github.com) reviewed

2023-11-17 15:47:15 -06:00

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

Its called every minute because its being run along with 2 other functions (Aussie Clock and Goob Schedule) which also run every minute

JMTNTBANG (Migrated from github.com) reviewed

2023-11-17 15:48:18 -06:00

aussie_clock.py

					
				@ -117,0 +129,4 @@

				                            await channel.send(f'Merry {today.year - b_day.year}th Birthmas {user.mention}!')

				                        else:

				                            await channel.send(f'Merry Birthmas {user.mention}!')

that will probably something i add in later on as that would require learning a new library, and im just now learning the json one

JMTNTBANG (Migrated from github.com) reviewed

2023-11-17 15:49:09 -06:00

commands/birthday.py

					
				@ -0,0 +1,61 @@

				import bot

yeah i tried that but had issues, but if that will work than thats what ill do

JMTNTBANG (Migrated from github.com) reviewed

2023-11-17 15:51:24 -06:00

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

yes the tuple comparison, that was something i was going for but forgot how to do it, thank you!

JMTNTBANG (Migrated from github.com) reviewed

2023-11-17 15:54:04 -06:00

birthdays.py

Forgot about that, ill have to remove that

JMTNTBANG commented

2023-11-17 16:05:32 -06:00

(Migrated from github.com)

I went through some of the changes and did commit some of them, others i will probably hold off until i have more information on how to actually do it

MaddyGuthridge (Migrated from github.com) reviewed

2023-11-17 23:19:15 -06:00

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

In that case, why check the same thing for every user whose birthday is today - you could just check the time at the start of the function

JMTNTBANG (Migrated from github.com) reviewed

2023-11-18 00:43:19 -06:00

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

ohh i see, move it outside the for loop, that way it saves processing power if it didn't need to do it in the first place

JMTNTBANG (Migrated from github.com) reviewed

2023-11-18 00:45:12 -06:00

aussie_clock.py

					
				@ -114,6 +115,22 @@ async def goob_schedule_upd():

				                        await channel.send(schedule_message)

i mean its already being checked with the actual date and time so i might as well keep it there

Sign in to join this conversation.

No reviewers

MaddyGuthridge

BobVonBob

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

JMTNTBANG/Bitey-Frank!29

				`@ -114,6 +115,22 @@ async def goob_schedule_upd():`
				`await channel.send(schedule_message)`

				`@ -0,0 +1,61 @@`
				`import bot`

				`@ -0,0 +1,61 @@`
				`import bot`

				`@ -0,0 +1,61 @@`
				`import bot`

Rows
Columns