Skip to content

feat: add ThreadAutoArchiveDuration enum #2826

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

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

Conversation

BOXERRMD
Copy link

@BOXERRMD BOXERRMD commented Jul 8, 2025

Add an enumeration ThreadAutoArchiveDuration to get time before the thread was archived.

The selected enumeration value is automatically retrieved. You can always set the archiving time as a number without using this enumeration.

Summary

This pull request doesn't make any corrections to the code, but adds an enumeration to help find the available thread archive times.
It's not necessarily necessary, but it makes the code easier to read and understand.

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

Add an enumeration ThreadAutoArchiveDuration to get time before the thread was archived.

The selected enumeration value is automatically retrieved. You can always set the archiving time as a number without using this enumeration.
Copy link
Member

@Paillat-dev Paillat-dev left a comment

Choose a reason for hiding this comment

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

I don't really see the benefit of this.

@BOXERRMD
Copy link
Author

BOXERRMD commented Jul 8, 2025

It just brings more understanding to the code. Yes, the modification isn't necessarily important, but it avoids having numbers that can be boring to read, and gives a better understanding of the time provided. You can even use enumeration to enable interactive modification with a bot without having to put in the four possibilities and just use enumeration.
(I'm trying to defend my idea because I had this problem at one point and I had to ask on the discord support to find out what archiving times were available :') )

@BOXERRMD
Copy link
Author

BOXERRMD commented Jul 8, 2025

I have apply changes ! :D

Copy link
Member

@Paillat-dev Paillat-dev left a comment

Choose a reason for hiding this comment

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

Also, please add the enum to the docs in https://github.com/Pycord-Development/pycord/blob/master/docs/api/enums.rst
Look at the other enums in the file and reproduce how they are declared.

@Dorukyum
Copy link
Member

Dorukyum commented Jul 8, 2025

I find this not very necessary. See the documentation and the type ThreadArchiveDuration.

ThreadArchiveDuration = Literal[60, 1440, 4320, 10080]

@BOXERRMD
Copy link
Author

BOXERRMD commented Jul 8, 2025

I find that just the figures as shown remain rather unclear about the actual time given. The addition I'm proposing doesn't add anything vital, but I think it makes the code easier to understand, without being compulsory.

@Paillat-dev
Copy link
Member

Paillat-dev commented Jul 8, 2025

I find this not very necessary.

Same to be fair, but as long as it's just an Enum class I feel like the changes are acceptable, unlike the original proposition which added too much verbosity. Although this is not strictly necessary.

Co-authored-by: Paillat <[email protected]>
Signed-off-by: BOXER <[email protected]>
@BOXERRMD
Copy link
Author

BOXERRMD commented Jul 8, 2025

👍 I have make changes !

Copy link
Member

@Paillat-dev Paillat-dev left a comment

Choose a reason for hiding this comment

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

As fare as code quality goes, lgtm.

From a usefulness perspective, I don't think this is strictly necessary but I feel like it's a plus to have.

@Dorukyum Dorukyum self-requested a review July 10, 2025 08:21
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

Also we have merge conflicts

@BOXERRMD
Copy link
Author

BOXERRMD commented Aug 2, 2025

Most imports are automatically modified by bots that reformat the code once the pull request is made

@Lulalaby
Copy link
Member

Lulalaby commented Aug 2, 2025

then disregard. But please fix the merge conflicts

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

Successfully merging this pull request may close these issues.

5 participants