Skip to content
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

logs id command #3196

Merged
merged 11 commits into from Jul 15, 2023
Merged

logs id command #3196

merged 11 commits into from Jul 15, 2023

Conversation

ghost
Copy link

@ghost ghost commented Sep 21, 2022

Added command logs id <key>, this helps users locate the log link / preview if they just have the log id.

This will help once a Heroku alternative is found and users log link changes due to moving from Heroku. They can use this command to bring up the new link easily

Have tested this.

Remade the PR because I changed the branch

@ghost ghost changed the base branch from master to development September 21, 2022 10:51
Copy link
Collaborator

@Taaku18 Taaku18 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Summarizing from my comments: I prefer if you rename the command from logs id -> logs key, and remove the open check from L1225 to L1232.

cogs/modmail.py Outdated Show resolved Hide resolved
cogs/modmail.py Outdated Show resolved Hide resolved
cogs/modmail.py Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Mar 14, 2023

Have completed the requested changes.

A closed log will show this
image

While if a log is open it will show this
image
I think this was the reason if logs were open we did not fetch the link.

Can see here it is a commented known bug
https://github.com/modmail-dev/modmail/blob/d6eba12937d4e0024e1c9208d0ac22b365829397/cogs/modmail.py#L758

Copy link
Collaborator

@Taaku18 Taaku18 left a comment

Choose a reason for hiding this comment

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

Everything else looks great, thanks for the changes.

cogs/modmail.py Outdated Show resolved Hide resolved
@ghost ghost requested a review from Taaku18 March 14, 2023 12:15
@StephenDaDev StephenDaDev requested review from Taaku18 and removed request for Taaku18 July 4, 2023 03:00
Taaku18
Taaku18 previously approved these changes Jul 11, 2023
@Taaku18 Taaku18 added the staged Staged for next version label Jul 11, 2023
@Taaku18 Taaku18 merged commit b1f3645 into modmail-dev:development Jul 15, 2023
@Taaku18 Taaku18 mentioned this pull request Jul 15, 2023
raidensakura pushed a commit to raidensakura/modmail that referenced this pull request Apr 10, 2024
* Update modmail.py

* Update clients.py

* Formatting

* Change log id to log key, added id as an alias

* Print the log even if it is closed and fix bug

* Update modmail.py

* Added a missing period

* Updated changelog

---------

Co-authored-by: Taku <45324516+Taaku18@users.noreply.github.com>
khakers pushed a commit to khakers/OpenModmail that referenced this pull request Jun 18, 2024
* Update modmail.py

* Update clients.py

* Formatting

* Change log id to log key, added id as an alias

* Print the log even if it is closed and fix bug

* Update modmail.py

* Added a missing period

* Updated changelog

---------

Co-authored-by: Taku <45324516+Taaku18@users.noreply.github.com>

(cherry picked from commit b1f3645)
Signed-off-by: Khakers <22665282+khakers@users.noreply.github.com>
khakers pushed a commit to khakers/OpenModmail that referenced this pull request Jun 18, 2024
* Update modmail.py

* Update clients.py

* Formatting

* Change log id to log key, added id as an alias

* Print the log even if it is closed and fix bug

* Update modmail.py

* Added a missing period

* Updated changelog

---------

Co-authored-by: Taku <45324516+Taaku18@users.noreply.github.com>

(cherry picked from commit b1f3645)
Signed-off-by: Khakers <22665282+khakers@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staged Staged for next version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants