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

feat(information): 新增校园资讯 #56

Merged
merged 13 commits into from
Aug 12, 2023

Conversation

Tong030905
Copy link
Collaborator

No description provided.

@j10ccc
Copy link
Collaborator

j10ccc commented Aug 6, 2023

你把 #55 关了干啥?提到的问题你也没改完啊

@j10ccc
Copy link
Collaborator

j10ccc commented Aug 6, 2023

校园资讯是确定了 专门做给 for you 发通知的是吗?

Copy link
Collaborator

@j10ccc j10ccc left a comment

Choose a reason for hiding this comment

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

我给你一个方案,你把 Information 的类型改了(这样子可能还要改 API),改成

interface Information {
  // ...other properties
  publisher: {
    name: string;
    backgroundImageUrl: string | null;
  }
}

背景图片是和发布者有直接关系的,而不是和一条推文有关,你写在这个组件里面是不合理的。这个关系应该是数据库控制的,而不是前端在代码里面绑定他们的关系。

这次给你留 Comment,你们对这种逻辑不关心的话就可以直接合并了 @Tianci-King

src/pages/announcement/InformationCard/index.vue Outdated Show resolved Hide resolved
@Tong030905
Copy link
Collaborator Author

新增了详情查询页面

src/pages/information/index.vue Outdated Show resolved Hide resolved
@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

informationList 其他地方也用不到,直接就一个计算属性好了,store 里面拿出来的也不用 reverse 了

@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

🥹,find 一下不会改 store 里面的值,就不需要额外解构了

@Tong030905
Copy link
Collaborator Author

🥹,find 一下不会改 store 里面的值,就不需要额外解构了

🥹🤪

@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

你看看两个 project.config.json 你这边能不能别提交上来

@Tong030905
Copy link
Collaborator Author

你看看两个 project.config.json 你这边能不能别提交上来

啊 我最近都只是提交src文档 这俩文件还被修改了吗🥹🥺

@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

你看看两个 project.config.json 你这边能不能别提交上来

啊 我最近都只是提交src文档 这俩文件还被修改了吗🥹🥺

这个 pr 对应的好几个 commit,你之前改的。你再发个 commit 改回来应该就行了

@Tong030905
Copy link
Collaborator Author

你看看两个 project.config.json 你这边能不能别提交上来

啊 我最近都只是提交src文档 这俩文件还被修改了吗🥹🥺

这个 pr 对应的好几个 commit,你之前改的。你再发个 commit 改回来应该就行了

🫡

@Tong030905
Copy link
Collaborator Author

你看看两个 project.config.json 你这边能不能别提交上来

啊 我最近都只是提交src文档 这俩文件还被修改了吗🥹🥺

这个 pr 对应的好几个 commit,你之前改的。你再发个 commit 改回来应该就行了

这下 是不是真的拿捏了 🥲

@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

pr 的 title 你改一下,有些时候合并用 squash,要重新写 commit message 的

@Tong030905
Copy link
Collaborator Author

pr 的 title 你改一下,有些时候合并用 squash,要重新写 commit message 的

没太明白,是改成squash?还是要改成什么🤔

@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

pr 的 title 你改一下,有些时候合并用 squash,要重新写 commit message 的

没太明白,是改成squash?还是要改成什么🤔

squash 是合并的一种方式,就是把所有 commit 合成一条新的。所以要写 commit message,内容是总结 pr 干了什么。pr 干了什么应该用一句话在 title 描述,你发 pr 的时候没注意。合并之前你先把 title 改了

@Tong030905 Tong030905 changed the title Info feat(information): 新增校园资讯 Aug 12, 2023
@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

你可以在自己仓库里面试一下 pr 的合并,比较 merge 和 squash 的区别

你这 pr 里面 commit 改糊了,美观起见我选择用 squash。squash 有个坏处就是所有细节没了,如果有多个开发者,那每个开发者干了什么也不知道了。所以尽量 commit 写规范,不重要的代码修改合并在重要的 commit 里面,你在自己的仓库处理好。

你这个 pr 就是后面写的代码有点小问题,应该自己在本地 rebase 改好再 force push 上来的。如果这样用 merge 是最好的(pr 中所有 commit 保留,同时会追加一条 merge commit),这次方便起见我直接在网页上 squash 了。

@Tianci-King 合并的时候注意这些

@j10ccc j10ccc closed this Aug 12, 2023
@j10ccc j10ccc reopened this Aug 12, 2023
@j10ccc j10ccc merged commit fc5643e into zjutjh:feat-information Aug 12, 2023
2 checks passed
@j10ccc
Copy link
Collaborator

j10ccc commented Aug 12, 2023

还有 pr 的标题和 issue 的标题没有格式一说,把内容描述一句话概括就行了,详细的描述写在 message body 里面(发pr 的时候应该有个大 textarea)

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