Skip to content

refactor: utils #14

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

Closed
wants to merge 54 commits into from
Closed

refactor: utils #14

wants to merge 54 commits into from

Conversation

Paillat-dev
Copy link
Member

@Paillat-dev Paillat-dev commented May 29, 2025

The general goal of these changes is to lower the amount of code we have to maintain across te library.

Changes include:

Separated utils function in:

  • utils.private for private utils that are to be used in the library code
  • utils for user facing utils

Removed over specific utils

Removed overly specific public utils that wouldn't commonly be used by users and aren't hard to re implement in any project. See CHANGELOG-V3.md for a comprehensive list.

Removed unused (or lesser used) utils

Remove utils that were used less than once or twice in the library in favor of:

  • Moving them to a specific file if they were specific to a part of the library and shouldn't have been in utils in the first place
  • For utils that implemented a one-liner, replace the util's usage with said one liner
  • For utils similar to other utils, use another util

Merged overlapping utils

Merge utils with common goals or with same goal with one another, e.g. see changes to utils.time_snowflake -> utils.generate_snowflake

@Paillat-dev Paillat-dev marked this pull request as draft May 29, 2025 18:30
* Start migration to uv

* Setup ruff and hatch

* Change pre-commit to use ruff

* Format with ruff

* Fix mistake

* Add dev deps

* Change workflows to use uv and ruff

* ➕ Add colorlog and remove requirements folder and fix build

* 💚 Fix sphinx build ?

* 🐛 Add __version.py for version management and update import in __init__.py

* ✏️ Update lib-checks.yml to run ruff on ubuntu-latest

* 🐛 Update lib-checks.yml to run mypy with uv

* 🔥 Delete MANIFEST.in

* ✨ Enhance lib-checks.yml to include ruff formatter check

* ♻️ Refactor pyproject.toml and uv.lock to use optional-dependencies for voice and speed
* chore: Update localization workflows to use 'uv' for dependency management

* chore: refactor Read the Docs configuration to use uv
@Paillat-dev Paillat-dev requested a review from Copilot June 24, 2025 13:56
Copilot

This comment was marked as outdated.

@Paillat-dev Paillat-dev requested a review from Copilot June 24, 2025 15:23
Copilot

This comment was marked as outdated.

@Paillat-dev Paillat-dev requested a review from Copilot July 7, 2025 20:38
Copilot

This comment was marked as outdated.

@Paillat-dev Paillat-dev marked this pull request as ready for review July 7, 2025 20:47
@Paillat-dev Paillat-dev requested review from Copilot and a team July 7, 2025 20:47
Copilot

This comment was marked as outdated.

@Paillat-dev Paillat-dev requested a review from Copilot July 7, 2025 20:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the utils module by splitting it into public (utils.public) and private (utils.private) submodules, removing obsolete helpers, and updating all code, documentation, and tests to use the new utilities.

  • Split discord/utils.py into utils/public.py (user-facing) and utils/private.py (internal), then removed the old monolith.
  • Replaced _get_as_snowflake, maybe_coroutine, utils.get, and other deprecated helpers with get_as_snowflake, maybe_awaitable, find, etc.
  • Updated imports across the codebase, adjusted docs and tests, and merged overlapping utilities (e.g. time_snowflakegenerate_snowflake).

Reviewed Changes

Copilot reviewed 62 out of 62 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_utils.py Updated imports to include generate_snowflake, but missing coverage for it
docs/locales/en/LC_MESSAGES/api/utils.po Translation reference for generate_snowflake is malformed
discord/member.py add_roles now uses a set comprehension, which loses original ordering
discord/ext/commands/converter.py Erroneous extra name=argument passed to find, breaking its API
discord/ext/tasks/init.py Duplicated compute_timedelta logic instead of reusing utils.private
Comments suppressed due to low confidence (5)

tests/test_utils.py:34

  • Add tests for generate_snowflake, covering both mode='boundary' (with high=True and high=False) and mode='realistic' to ensure it produces correct snowflake values.
    generate_snowflake,

docs/locales/en/LC_MESSAGES/api/utils.po:616

  • This reference is malformed; update it to refer to discord.utils.generate_snowflake instead of discord.utils.:1.
#: 3dbe398f94684d7d81111c3a9d78ddee discord.utils.:1 of

discord/ext/commands/converter.py:794

  • The extra keyword argument name=argument is invalid for find, which only takes a predicate and a sequence; remove name=argument.
                result = discord.utils.find(lambda e: e.name == argument, guild.emojis)

discord/member.py:1051

  • Using a set comprehension here drops the original order of roles; consider using dict.fromkeys(...) or utils._unique to dedupe while preserving order.
            new_roles = list({Object(id=r.id) for s in (self.roles[1:], roles) for r in s})

discord/ext/tasks/init.py:52

  • [nitpick] This duplicates the same logic in utils.private.compute_timedelta; consider importing and reusing that function to avoid code duplication.
def compute_timedelta(dt: datetime.datetime):

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.

1 participant