Skip to content

在使用过程中发现的一些小优化 #4

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

Merged
merged 5 commits into from
Dec 16, 2023
Merged

在使用过程中发现的一些小优化 #4

merged 5 commits into from
Dec 16, 2023

Conversation

Endeavour233
Copy link
Contributor

首先感谢开发这个工具,用起来很方便。在使用过程中觉得是否可以进行以下几点优化:

  1. repo的README.md文件仅用来说明别人可以怎样使用这个repo来管理自己的刷题记录,刷题记录保存在另一个文件DailyLC.md中
  2. 使用中发现,若对已有issue进行编辑,在由此触发的对README文件的更新过程中,有把Calendar行误识别为记录行的可能性。比如编辑issue number为4的issue, 会将Calendar section下2023/12/4所在那行识别成4号issue对应的记录行(对应fix:main.py 3664a3b
  3. 使用中发现,若对已有issue进行编辑,比如编辑标题,更新后的README文件会不太对(例子)(对应fix: main.py f80825f)
  4. 个人觉得随着以后更多人使用这些工具,可能会提一些针对工具本身的真正的issue,这些issue需要和那些用来记录刷题的issue作区分,所以觉得是否可以用“issue标题以R开头“作为记录刷题的issue的标识符,只有这些issue会触发对README(即DailyLC)和backup文件夹的更新(对应代码: main.py ed8318b 665818c)

@Doragd
Copy link
Owner

Doragd commented Dec 11, 2023

@Endeavour233 感谢你的pr,提了非常好的建议和修复了目前好几个问题,这周内我仔细review下再merge~

@Doragd Doragd merged commit 10907ae into Doragd:main Dec 16, 2023
@Doragd
Copy link
Owner

Doragd commented Dec 16, 2023

@Endeavour233 实际上workflow里面区分了 自己刷题的issue和别人提问题的issue:github.repository_owner_id == github.event.issue.user.id。我先merge了,然后修复好。 给工具使用者增加多余劳动是不好的,比如限制前缀是R

@Doragd
Copy link
Owner

Doragd commented Dec 16, 2023

对已有issue进行编辑,就会覆盖掉原来同issue number的,这个是符合预期的呀 @Endeavour233

@Endeavour233
Copy link
Contributor Author

对已有issue进行编辑,就会覆盖掉原来同issue number的,这个是符合预期的呀 @Endeavour233

关于这一条,我指的是没有正确覆盖,比如下面这个截图是我之前对#3 issue进行编辑后,readme文件的diff, 可以看到新的25行的最后多了不应该出现的1列日期。我用“重新以w模式打开file然后再写”替换原来的“通过seek(0)回到file开头写”尝试进行了修复
截屏2023-12-16 19 37 45

@Endeavour233
Copy link
Contributor Author

@Endeavour233 实际上workflow里面区分了 自己刷题的issue和别人提问题的issue:github.repository_owner_id == github.event.issue.user.id。我先merge了,然后修复好。 给工具使用者增加多余劳动是不好的,比如限制前缀是R

原来如此

@Doragd
Copy link
Owner

Doragd commented Dec 16, 2023

对已有issue进行编辑,就会覆盖掉原来同issue number的,这个是符合预期的呀 @Endeavour233

关于这一条,我指的是没有正确覆盖,比如下面这个截图是我之前对#3 issue进行编辑后,readme文件的diff, 可以看到新的25行的最后多了不应该出现的1列日期。我用“重新以w模式打开file然后再写”替换原来的“通过seek(0)回到file开头写”尝试进行了修复 截屏2023-12-16 19 37 45

sorry, 我miss了这件事,马上修复

@Doragd
Copy link
Owner

Doragd commented Dec 16, 2023

@Endeavour233 已修复

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