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

增加更多组件 #12

Closed
wants to merge 17 commits into from
Closed

增加更多组件 #12

wants to merge 17 commits into from

Conversation

mei-rune
Copy link

@mei-rune mei-rune commented Mar 1, 2019

fix #11
我已经尽可能地将 风格类的修改 revert 了,你看看还有什么要修的
注意 src/components 下的 vue 是我直接从 vue-element-admin 中拷过来的,有一部分改了,有一部分还没有改,我正在慢慢改, 请先合并吧, 然后再慢慢改

@mei-rune mei-rune force-pushed the master branch 3 times, most recently from bc7225a to 61ad700 Compare March 1, 2019 12:48
@Armour Armour self-requested a review March 4, 2019 16:50
@Armour Armour added the enhancement New feature or request label Mar 4, 2019
.eslintignore Outdated Show resolved Hide resolved
Copy link
Owner

@Armour Armour left a comment

Choose a reason for hiding this comment

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

Looks awesome in general, but there are still lots of work need to be done :)
It would be great if you can split this BIG PR into many small PR so that we can gradually merge different new features.
Again, thanks for your time! 🎉

.eslintignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
mock/article.ts Outdated Show resolved Hide resolved
mock/article.ts Outdated Show resolved Hide resolved
mock/article.ts Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
@@ -1,20 +0,0 @@
{
Copy link
Owner

Choose a reason for hiding this comment

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

Why remove this file? Is this one not used in the project?

Copy link
Author

Choose a reason for hiding this comment

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

replace tslint with eslint, becase tslint is deprecated

vue.config.js Outdated Show resolved Hide resolved
pwa: {
name: 'vue-typescript-admin-template'
},

configureWebpack: {
Copy link
Owner

Choose a reason for hiding this comment

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

Better make sure each of below config is useful or need to be set in the project.

@mei-rune
Copy link
Author

mei-rune commented Mar 5, 2019

你先别 review 我在代码风格这方面还没弄好

@Armour
Copy link
Owner

Armour commented Mar 5, 2019

不着急 我先review一下给你个方向 你慢慢来 我最近也忙

@mei-rune
Copy link
Author

修改暂时告一段落, 应该还有很多bugs, 这些bug我会在后续从 vue-element-admin 中拷页面时修复。
关于风格问题, 请用 eslint --fix 来修复吧

我希望能快点合并, 这样我可以做后续工作,将更多的 vue-element-admin 中页面拷过来

@Armour
Copy link
Owner

Armour commented Mar 14, 2019

你应该在自己的branch把东西测试好了 没有bug了再申请合并 这是基本的流程 我之所以没有马上合并的原因是因为改动还有不少有待商榷的地方 我会在code review里都给你提出来 需要确保一些有问题的地方都fix了才可以合并 毕竟这是一次很大的改动 还是要保重code quality的

(其实我之前有推荐你不要一次给这么大的改动 而是应该分成很多小的PR一点点来 保证每次改动小而精确 这样我们都会方便点 code也会质量高一些 现在这样就需要慢慢来了 不要太着急 总之还是很谢谢你的contribute的 :)

@Armour
Copy link
Owner

Armour commented Mar 21, 2019

#40

@Armour
Copy link
Owner

Armour commented Apr 24, 2019

Closed due to inactivity.

@Armour Armour closed this Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants