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.

Sign up for GitHub

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

Jump to bottom

fix: support pnpm #57

Merged
merged 16 commits into from
Mar 21, 2023
Merged

fix: support pnpm #57

merged 16 commits into from
Mar 21, 2023

Conversation

conglinyizhi
Copy link
Contributor

@conglinyizhi conglinyizhi commented Mar 18, 2023 •
edited
Loading

  • Fix the problem of not being able to install with pnpm install (fixed by installing several packages)
  • Adding a few TypeScript comments will help more with syntax hints in the editor
  • Try to make a click-to-copy switch function and turn it off with default (for the time being, it will be operated with a button, but we will consider using the Switch component instead) and test it locally
  • A configuration was added to the .gitignore file to tell git to ignore pnpm's locked files

In the future, can I fill in information in Chinese? Just when submitting a pull request. As Chinese can better express my intentions, English may lead to semantic bias, I look forward to your reply.

  • 修复 不能使用 pnpm 安装的问题（解决方式是新增加了几个软件包）
  • 新增 点击摘要功能的开关
  • 在 .gitignore 文件中忽略 pnpm 的 lock 文件
  • 在部分位置增加 TypeScirpt 类型注释，用于更好的编辑器类型提示

JimmyLv
JimmyLv requested changes Mar 19, 2023
View reviewed changes
Copy link
Owner

@JimmyLv JimmyLv left a comment

Choose a reason for hiding this comment

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

sure, you can use Chinese. by the way, could you please change the PR title by following the https://semver.org/?

Also, the location of this button, expect it can be placed elsewhere 😁 deployment here: https://chat-bilibili-video-6xhd7kab2-jimmy-lv-s-team.vercel.app/

Google Chrome-httpschat-bilibili-video-6xhd7kab2-jimmy-lv-s-team vercel app-001747-20230319

@conglinyizhi
Copy link
Contributor Author

好的，我会尽快应用这些修改

@conglinyizhi conglinyizhi changed the title Some changes Some changes 1.6.1 Mar 19, 2023
@conglinyizhi conglinyizhi changed the title Some changes 1.6.1 1.6.1 Mar 19, 2023
@conglinyizhi
Copy link
Contributor Author

完成，按钮已经更换为一个基于 Swicth 组建封装的 SwicthItem 并且更换了放置位置。

我不认为新增一个开关是新的功能，因此，我没有增加次版本号

@conglinyizhi conglinyizhi requested a review from JimmyLv March 19, 2023 05:42
JimmyLv
JimmyLv requested changes Mar 19, 2023
View reviewed changes
Copy link
Owner

@JimmyLv JimmyLv left a comment

Choose a reason for hiding this comment

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

PR的标题用于描述功能，比如 feat: add copy switch button 或 fix: support pnpm
最终会变成一个 git commit，这个 版本号不用你自己操作的，由 GitHub Action 自动生成。
另外麻烦把一些没必要的代码注释去掉哇， // thanks

@conglinyizhi conglinyizhi changed the title 1.6.1 fix: support pnpm Mar 19, 2023
@conglinyizhi
Copy link
Contributor Author

conglinyizhi commented Mar 19, 2023 •
edited
Loading

PR的标题用于描述功能，比如 feat: add copy switch button 或 fix: support pnpm 最终会变成一个 git commit，这个 版本号不用你自己操作的，由 GitHub Action 自动生成。 另外麻烦把一些没必要的代码注释去掉哇， // thanks

我已经修改了 PR 的标题，但是我不清楚您描述的“没必要的代码”指的是哪些，是类似图片中的这种可以让编辑器展示更多提示的注释吗？

  • 如果是，请告知我，我在未来的提交中可以尽可能不放置这些注释，并且在稍后会删除他们
  • 或者，您是说在 hooks/notes/lark.ts 中的 but why 注释？这个注释的原因是我目前不明白传入的数据中会多出来一个换行符，这个开头的换行符会导致问题 有些时候导出到飞书的内容前面会有一行空白，强迫症用户看着稍微有点难受 #54 ，所以我在某次修改的时候加入了这一行注释，考虑到这行注释可能会对其他贡献者产生麻烦，我也可以删除。

期待您的回复。

图片

@conglinyizhi conglinyizhi requested a review from JimmyLv March 19, 2023 09:41
JimmyLv
JimmyLv requested changes Mar 19, 2023
View reviewed changes
Copy link
Owner

@JimmyLv JimmyLv left a comment

Choose a reason for hiding this comment

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

辛苦再修改一下下

components/ActionsAfterResult.tsx Outdated
@@ -19,6 +19,7 @@ export function ActionsAfterResult({
const { loading: flomoLoading, save: flomoSave } = useSaveToFlomo(summaryNote, flomoWebhook || '')
const { loading: larkLoading, save: larkSave } = useSaveToLark(summaryNote, larkWebhook || '')
const hasNoteSetting = flomoWebhook || larkWebhook
// const [clickCopy, setClickCopy] = useLocalStorage<boolean>('user-config-clickCopy') || false
Copy link
Owner

Choose a reason for hiding this comment

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

哈哈哈 我指的这个啦

components/SummaryResult.tsx Outdated
import { formatSummary } from '~/utils/formatSummary'
// import { Switch as SwitchItem } from './ui/switch'
Copy link
Owner

Choose a reason for hiding this comment

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

here

components/SummaryResult.tsx Outdated
<div
className="mx-auto mt-6 max-w-3xl cursor-copy rounded-xl border-2 bg-white p-4 text-lg leading-7 shadow-md transition hover:bg-gray-50"
onClick={handleCopy}
onClick={clickCopyTest}
Copy link
Owner

Choose a reason for hiding this comment

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

xxxTest 可不是好的名字

hooks/notes/lark.ts
content: [
[
{
tag: 'text',
Copy link
Owner

Choose a reason for hiding this comment

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

其实可以用'markdown' 你看看飞书的文档哇？

@conglinyizhi
Copy link
Contributor Author

conglinyizhi commented Mar 20, 2023 via email •
edited
Loading

@conglinyizhi conglinyizhi requested a review from JimmyLv March 20, 2023 13:15
@JimmyLv JimmyLv merged commit 65a312c into JimmyLv:main Mar 21, 2023
@JimmyLv JimmyLv mentioned this pull request Mar 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JimmyLv JimmyLv JimmyLv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
BibiGPT Roadmap
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@conglinyizhi @JimmyLv