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

添加处理批量工单功能 #2658

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

Conversation

cslingjun
Copy link
Contributor

@cslingjun cslingjun commented May 21, 2024

sql/sql_workflow.py Fixed Show fixed Hide fixed
Copy link

codecov bot commented May 21, 2024

Codecov Report

Attention: Patch coverage is 28.00000% with 72 lines in your changes are missing coverage. Please review.

Project coverage is 76.80%. Comparing base (14fbd5e) to head (ce36f74).
Report is 1 commits behind head on master.

Files Patch % Lines
sql/sql_workflow.py 26.13% 65 Missing ⚠️
sql/views.py 41.66% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2658      +/-   ##
==========================================
- Coverage   77.11%   76.80%   -0.32%     
==========================================
  Files         117      117              
  Lines       16170    16259      +89     
==========================================
+ Hits        12470    12487      +17     
- Misses       3700     3772      +72     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@LeoQuote LeoQuote left a comment

Choose a reason for hiding this comment

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

  1. 代码复用不够, 这些检查的代码在单个工单的审核和执行中有类似的, 希望能复用一部分, 可以考虑直接把代码挪到 model 层, 作为 workflow 的一个 方法来做
  2. 请不要再为了同样的case重开pr,会损失之前的讨论
  3. 审核代码已重构,请考虑使用全新的AuditV2

如果有其他的建议或困难欢迎提出

@cslingjun
Copy link
Contributor Author

cslingjun commented May 21, 2024

  1. 代码复用不够, 这些检查的代码在单个工单的审核和执行中有类似的, 希望能复用一部分, 可以考虑直接把代码挪到 model 层, 作为 workflow 的一个 方法来做
  2. 请不要再为了同样的case重开pr,会损失之前的讨论
  3. 审核代码已重构,请考虑使用全新的AuditV2

如果有其他的建议或困难欢迎提出

  1. 代码复用不够, 这些检查的代码在单个工单的审核和执行中有类似的, 希望能复用一部分, 可以考虑直接把代码挪到 model 层, 作为 workflow 的一个 方法来做
  2. 请不要再为了同样的case重开pr,会损失之前的讨论
  3. 审核代码已重构,请考虑使用全新的AuditV2

如果有其他的建议或困难欢迎提出

1、这块不太明白,能否指导下 ?
2、重开pr,是有很多哥们私我这块功能,然后我这边也不是开发,git用的比较少,不太会玩,抱歉了

Copy link
Collaborator

@LeoQuote LeoQuote left a comment

Choose a reason for hiding this comment

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

我重新细看了下当前的逻辑,复用方面确实跟以前不一样,提交和取消都是复用了单个工作流的代码,也复用了单个工作流的model,只是多了一个虚的总工单。
总体上对已有的工单代码修改不大,甚至也没有多少的侵入性,不想用这个功能的人不用就行了。

我唯一的建议变成了给代码加上一些单元测试,这有利于代码和功能的长期发展,还希望大佬考虑一下。
以及谢谢大佬的贡献!

@cslingjun
Copy link
Contributor Author

我重新细看了下当前的逻辑,复用方面确实跟以前不一样,提交和取消都是复用了单个工作流的代码,也复用了单个工作流的model,只是多了一个虚的总工单。 总体上对已有的工单代码修改不大,甚至也没有多少的侵入性,不想用这个功能的人不用就行了。

我唯一的建议变成了给代码加上一些单元测试,这有利于代码和功能的长期发展,还希望大佬考虑一下。 以及谢谢大佬的贡献!

单元测试就真不太会了 ,大佬帮忙写一点?这边功能我手动测试是没问题了

@LeoQuote
Copy link
Collaborator

暂时没空帮忙写单元测试,等我什么时候有空吧,欢迎其他人来帮忙补充单元测试

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.

2 participants