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

chore: use father #268

Merged
merged 21 commits into from Aug 31, 2020
Merged

chore: use father #268

merged 21 commits into from Aug 31, 2020

Conversation

xrkffgg
Copy link
Member

@xrkffgg xrkffgg commented Aug 21, 2020

No description provided.

@vercel
Copy link

vercel bot commented Aug 21, 2020

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/react-component/upload/5oh00dkmo
✅ Preview: https://upload-git-up-father.react-component.vercel.app

@xrkffgg xrkffgg requested review from afc163 and zombieJ August 21, 2020 03:11
@lgtm-com
Copy link

lgtm-com bot commented Aug 21, 2020

This pull request introduces 2 alerts and fixes 2 when merging 5da11b7 into 472a3d6 - view on LGTM.com

new alerts:

  • 2 for Hard-coded credentials

fixed alerts:

  • 2 for Hard-coded credentials

@lgtm-com
Copy link

lgtm-com bot commented Aug 21, 2020

This pull request introduces 2 alerts and fixes 2 when merging e9eaa8e into 472a3d6 - view on LGTM.com

new alerts:

  • 2 for Hard-coded credentials

fixed alerts:

  • 2 for Hard-coded credentials

@zombieJ
Copy link
Member

zombieJ commented Aug 21, 2020

@kermit-xuan could you help to review?

@kerm1it
Copy link
Member

kerm1it commented Aug 21, 2020

image

@xrkffgg please install it.

package.json Outdated Show resolved Hide resolved
@kerm1it
Copy link
Member

kerm1it commented Aug 21, 2020

@zombieJ 是不是考虑加个 compile 的 CI 检查?就和 antd 的一样

@zombieJ
Copy link
Member

zombieJ commented Aug 22, 2020

@zombieJ 是不是考虑加个 compile 的 CI 检查?就和 antd 的一样

需要的,迁移 father 都要加一个~

@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2020

This pull request introduces 2 alerts and fixes 2 when merging f018c70 into 472a3d6 - view on LGTM.com

new alerts:

  • 2 for Hard-coded credentials

fixed alerts:

  • 2 for Hard-coded credentials

@xrkffgg
Copy link
Member Author

xrkffgg commented Aug 24, 2020

@kermit-xuan PTAL

@kerm1it
Copy link
Member

kerm1it commented Aug 24, 2020

可以在 CI 的 lint 后面加个 compile 的检查么?防止编译出错

.travis.yml Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2020

This pull request introduces 2 alerts and fixes 2 when merging 8eb6da1 into 472a3d6 - view on LGTM.com

new alerts:

  • 2 for Hard-coded credentials

fixed alerts:

  • 2 for Hard-coded credentials

Copy link
Member

@kerm1it kerm1it left a comment

Choose a reason for hiding this comment

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

测试不需要改造的话,我感觉OK了。

import axios from 'axios';
import Upload from '../src/index';
Copy link
Member

@afc163 afc163 Aug 24, 2020

Choose a reason for hiding this comment

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

Suggested change
import Upload from '../src/index';
import Upload from '..';

@afc163
Copy link
Member

afc163 commented Aug 24, 2020

覆盖率的 ci 是不是没生效。

@kerm1it
Copy link
Member

kerm1it commented Aug 24, 2020

运行了,但是好像没有打印出来。

.travis.yml Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2020

This pull request introduces 2 alerts and fixes 2 when merging 972147f into 472a3d6 - view on LGTM.com

new alerts:

  • 2 for Hard-coded credentials

fixed alerts:

  • 2 for Hard-coded credentials

@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2020

This pull request introduces 2 alerts and fixes 2 when merging 225ce2b into 472a3d6 - view on LGTM.com

new alerts:

  • 2 for Hard-coded credentials

fixed alerts:

  • 2 for Hard-coded credentials

@lgtm-com
Copy link

lgtm-com bot commented Aug 29, 2020

This pull request introduces 1 alert and fixes 3 when merging d32f18b into bb07467 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

fixed alerts:

  • 2 for Hard-coded credentials
  • 1 for Useless conditional

@kerm1it
Copy link
Member

kerm1it commented Aug 29, 2020

应该差不多了,review 的时候最好在本地编辑器看,改动有点大(类型定义)。

@xrkffgg xrkffgg changed the title [WIP] chore: use father chore: use father Aug 30, 2020
@@ -1,4 +1,3 @@
// export this package's api
import Upload from './Upload';
Copy link
Member

Choose a reason for hiding this comment

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

多加一个 UploadProps export.

},
{},
);
const dataOrAriaAttributeProps = (props: React.AriaAttributes | React.DataHTMLAttributes<any>) => {
Copy link
Member

Choose a reason for hiding this comment

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

等一下这个:react-component/util#142

Copy link
Member

Choose a reason for hiding this comment

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

done:

+ rc-util@5.2.0

pickerAttrs(props, { aria: true, data: true });

Copy link
Member Author

Choose a reason for hiding this comment

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

这个是改啥了?

Copy link
Member

Choose a reason for hiding this comment

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

就是直接用 rc-util 里面的 pickerAttrs,这个方法可以删掉了。

Copy link
Member Author

Choose a reason for hiding this comment

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

这个怎么用呢?😂


componentDidMount() {
this._isMounted = true;
}

componentWillUnmount() {
this._isMounted = false;
this.abort();
this.abort('');
Copy link
Member

@zombieJ zombieJ Aug 31, 2020

Choose a reason for hiding this comment

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

是不是可以 this.abort(null)?

下面的 abort 定义让 file 变成 optional 的即可~

@kerm1it
Copy link
Member

kerm1it commented Aug 31, 2020

我在考虑要不要加个 RCFile 的定义? 不然用到 uid 属性的地方就都得变成any。

@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2020

This pull request introduces 1 alert and fixes 3 when merging 1e515ba into bb07467 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

fixed alerts:

  • 2 for Hard-coded credentials
  • 1 for Useless conditional

@zombieJ
Copy link
Member

zombieJ commented Aug 31, 2020

我在考虑要不要加个 RCFile 的定义? 不然用到 uid 属性的地方就都得变成any。

👍 的确 any 有点蛋疼

@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2020

This pull request introduces 1 alert and fixes 3 when merging ef2ebbc into bb07467 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

fixed alerts:

  • 2 for Hard-coded credentials
  • 1 for Useless conditional

@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2020

This pull request introduces 1 alert and fixes 3 when merging cfcf2d2 into bb07467 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

fixed alerts:

  • 2 for Hard-coded credentials
  • 1 for Useless conditional

@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2020

This pull request fixes 3 alerts when merging a58f859 into bb07467 - view on LGTM.com

fixed alerts:

  • 2 for Hard-coded credentials
  • 1 for Useless conditional

@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2020

This pull request fixes 3 alerts when merging b8538d6 into bb07467 - view on LGTM.com

fixed alerts:

  • 2 for Hard-coded credentials
  • 1 for Useless conditional

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.

None yet

4 participants