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

[CodeStyle] add introducing isort #331

Merged
merged 7 commits into from
Nov 24, 2022
Merged

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Nov 12, 2022

@paddle-bot
Copy link

paddle-bot bot commented Nov 12, 2022

你的PR提交成功,感谢你对开源项目的贡献!
请检查PR提交格式和内容是否完备,具体请参考示例模版
Your PR has been submitted. Thanks for your contribution!
Please check its format and content. For this, you can refer to Template and Demo.

@luotao1 luotao1 self-assigned this Nov 14, 2022
rfcs/CodeStyle/20221111_introducing_isort.md Show resolved Hide resolved
rfcs/CodeStyle/20221111_introducing_isort.md Outdated Show resolved Hide resolved
rfcs/CodeStyle/20221111_introducing_isort.md Outdated Show resolved Hide resolved
rfcs/CodeStyle/20221111_introducing_isort.md Outdated Show resolved Hide resolved
rfcs/CodeStyle/20221111_introducing_isort.md Show resolved Hide resolved
rfcs/CodeStyle/20221111_introducing_isort.md Show resolved Hide resolved

### 推进方式

第一个 PR(即 [PaddlePaddle/Paddle#46475](https://github.com/PaddlePaddle/Paddle/pull/46475))添加配置并修复部分文件,之后每个 PR 修复一部分文件(尽可能保持在 1000 左右,以避免 PR-CI-Coverage 流水线崩溃在参数传递处),利用大概 5 个以内 PR 修复绝大多数文件格式,之后利用一些 PR 专注于解决需要手动解决的问题。
Copy link
Collaborator

Choose a reason for hiding this comment

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

利用大概 5 个以内 PR 修复绝大多数文件格式

来自 @jeff41404 的建议,具体实施时建议找个不太冲突的时间集中改

Copy link
Member Author

Choose a reason for hiding this comment

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

目前 PaddlePaddle/Paddle#46475 过去四天了,只有三个冲突,其实一般情况下不专门找时间应该也能合入(任意晚上提 PR,第二天白天 merge),集中找时间改还需要保证各个 PR 之间互不干涉,感觉有点麻烦

前两个 PR 主要修改单测应该冲突概率都不高,第三个及之后 PR 可能主要是其他部分,这部分可能冲突概率高一些,如果确实冲突概率高的话,可以考虑那部分再找时间来改

@luotao1
Copy link
Collaborator

luotao1 commented Nov 18, 2022

@SigureMo 本次评审不再单独增设会议,采用邮件(已发)+ 讨论的方式进行评审

  • 有任何问题,大家都会在本 PR 上直接进行评论
  • 如果没有问题,将在11月28日合入本 PR,并按照评审方案进行开发

@luotao1
Copy link
Collaborator

luotao1 commented Nov 24, 2022

@SigureMo@XiaoguangHu01 @raindrops2sea 讨论,评审没有问题,可以现在就启动代码合入了。

@luotao1 luotao1 merged commit 5f167ea into PaddlePaddle:master Nov 24, 2022
@SigureMo SigureMo deleted the rfc/isort branch November 24, 2022 05:46
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.

3 participants