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

fix: typo in clean messages periodical task's logging #12090

Merged
merged 1 commit into from
Dec 26, 2024

Conversation

bowenliang123
Copy link
Contributor

@bowenliang123 bowenliang123 commented Dec 25, 2024

Summary

  • fix typo in logging of Cleaned unused dataset to Cleaned messages in clean_message periodical task
  • remove the unused variable page

Checklist

Important

Please review the checklist below before submitting your pull request.

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. 🐞 bug Something isn't working labels Dec 25, 2024
@bowenliang123
Copy link
Contributor Author

cc @JohnJyong @laipz8200

@bowenliang123 bowenliang123 force-pushed the improve-clean-messages branch 4 times, most recently from 61f47c1 to a76aef7 Compare December 25, 2024 16:25
@laipz8200 laipz8200 requested a review from JohnJyong December 26, 2024 04:10
@JohnJyong JohnJyong closed this Dec 26, 2024
@bowenliang123
Copy link
Contributor Author

Hi @JohnJyong, may I ask for your advice to improve this PR? And what's the main reason for closing this PR?

@laipz8200
Copy link
Member

Hi @bowenliang123. Sorry for close this PR directly, because this task is related to Cloud services. At the moment, we believe it does not require any changes.

@bowenliang123
Copy link
Contributor Author

Thanks for keeping me informed.

Or may I strip changes and only fix the typo in logging, if you can reopen this PR ? Cleaned unused dataset is not correct in log text anyway.

@laipz8200
Copy link
Member

only fix the typo in logging

SGTM.

@laipz8200 laipz8200 reopened this Dec 26, 2024
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Dec 26, 2024
@bowenliang123 bowenliang123 changed the title fix: improve clean outdated messages periodical task fix: typo in clean messages periodical task's logging Dec 26, 2024
@bowenliang123
Copy link
Contributor Author

Thanks @laipz8200 . The code and the PR's description have been updated, narrowed to minor fix in logging text and unused variable. May I ask for a second round of review in this , @JohnJyong ?

Copy link
Member

@laipz8200 laipz8200 left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 26, 2024
@laipz8200 laipz8200 merged commit 886758d into langgenius:main Dec 26, 2024
5 checks passed
@laipz8200
Copy link
Member

@JohnJyong and I reviewed this change together. Thank you for your contribution!

@bowenliang123 bowenliang123 deleted the improve-clean-messages branch December 26, 2024 09:44
@bowenliang123
Copy link
Contributor Author

Cool. I'm always glad to have Dify team's feedback and advices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants