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: add axios request test #14

Merged
merged 10 commits into from
Jan 15, 2022
Merged

Conversation

shadowdreamer
Copy link
Contributor

@shadowdreamer shadowdreamer commented Jan 14, 2022

加了个axios请求库,写法很土,环境变量是放在对应workplace下面吗

两个问题

@vercel
Copy link

vercel bot commented Jan 14, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/bangumi-fe/frontend/5NxBnxTzXcULS2e2kTU5oHeBS3cb
✅ Preview: https://frontend-git-fork-shadowdreamer-master-bangumi-fe.vercel.app

@vercel vercel bot temporarily deployed to Preview January 14, 2022 16:27 Inactive
@shadowdreamer
Copy link
Contributor Author

打包之后浏览器会报 Uncaught TypeError: r.exports.jsxDEV is not a function 先睡了……

@vercel vercel bot temporarily deployed to Preview January 14, 2022 16:52 Inactive
@vercel vercel bot temporarily deployed to Preview January 15, 2022 02:39 Inactive
@codecov
Copy link

codecov bot commented Jan 15, 2022

Codecov Report

Merging #14 (814dd38) into master (ca86bef) will decrease coverage by 12.93%.
The diff coverage is 75.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##            master      #14       +/-   ##
============================================
- Coverage   100.00%   87.06%   -12.94%     
============================================
  Files            3        5        +2     
  Lines           58      116       +58     
  Branches         3        7        +4     
============================================
+ Hits            58      101       +43     
- Misses           0       14       +14     
- Partials         0        1        +1     
Impacted Files Coverage Δ
packages/website/src/App.tsx 79.16% <62.96%> (-20.84%) ⬇️
packages/website/src/api/request.ts 79.16% <79.16%> (ø)
packages/website/src/api/character.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca86bef...814dd38. Read the comment docs.

Copy link
Contributor

@Ayase-252 Ayase-252 left a comment

Choose a reason for hiding this comment

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

谢谢贡献

packages/website/src/types/common.ts Outdated Show resolved Hide resolved

// 异常拦截处理器
const errorHandler = (error:any) => {
console.log(error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log(error)

console.log 好像不太必要

Copy link
Contributor Author

Choose a reason for hiding this comment

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

请求异常先log一下,以后再补异常处理

packages/website/src/api/character.ts Outdated Show resolved Hide resolved
@Ayase-252
Copy link
Contributor

Ayase-252 commented Jan 15, 2022

bgm38,如果可以的话麻烦补个单测吗?不行的话我来补也可以,当作后来 PR 的一个例子。

@shadowdreamer
Copy link
Contributor Author

bgm38,如果可以的话麻烦补个单测吗?不行的话我来补也可以,当作后来 PR 的一个例子。

没写过单测,不知道怎么写b38

@vercel vercel bot temporarily deployed to Preview January 15, 2022 05:39 Inactive
@vercel vercel bot temporarily deployed to Preview January 15, 2022 05:40 Inactive
packages/website/src/api/character.ts Outdated Show resolved Hide resolved
packages/website/src/App.tsx Outdated Show resolved Hide resolved
Comment on lines 5 to 7
const api = {
getCharacterDetail: '/v0/characters/'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

个人觉得可以不用这个 api,直接写到方法上就好。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

有的时候可以复用,路径多了也方便管理

Copy link
Contributor Author

Choose a reason for hiding this comment

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

又发邮件警报了 我删掉了🤣

packages/website/src/api/character.ts Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview January 15, 2022 06:55 Inactive
@vercel vercel bot temporarily deployed to Preview January 15, 2022 06:56 Inactive
@vercel vercel bot temporarily deployed to Preview January 15, 2022 07:41 Inactive
@vercel vercel bot temporarily deployed to Preview January 15, 2022 10:00 Inactive
@Ayase-252 Ayase-252 merged commit ed8bcfd into bangumi:master Jan 15, 2022
@Ayase-252
Copy link
Contributor

Merged, thanks

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 14, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9b94e66
Status: ✅  Deploy successful!
Preview URL: https://a7ade082.bangumi-frontend.pages.dev
Branch Preview URL: https://pr-627-storybook.bangumi-frontend.pages.dev

View logs

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