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

Add Import csv.md & blank line argument #1192

Merged
merged 12 commits into from
Dec 4, 2022
Merged

Add Import csv.md & blank line argument #1192

merged 12 commits into from
Dec 4, 2022

Conversation

wycccccc
Copy link
Collaborator

resolve #1114

@wycccccc wycccccc requested a review from chia7712 November 30, 2022 05:57
@chia7712
Copy link
Contributor

我剛剛才注意到 (https://github.com/skiptests/astraea/blob/main/app/src/main/java/org/astraea/app/backup/ImportCsv.java#L112) 這邊可以跳過幾行不是參數化控制，可否修正一下？然後將該參數補到文件裡面

@wycccccc
Copy link
Collaborator Author

這邊可以跳過幾行不是參數化控制，可否修正一下？然後將該參數補到文件裡面

是規定要跳過開始的哪幾行就行了對吧。

@chia7712
Copy link
Contributor

是規定要跳過開始的哪幾行就行了對吧。

是的，一個整數值，預設為0 (不跳過)

@wycccccc
Copy link
Collaborator Author

好 修改完畢

@wycccccc
Copy link
Collaborator Author

wycccccc commented Dec 1, 2022 •
edited
Loading

暫時有問題還需要修一下
添加 #1114 後續討論中 處理最後一行blank line問題邏輯

@wycccccc
Copy link
Collaborator Author

wycccccc commented Dec 2, 2022

修復完畢 && resolve conflict

@wycccccc
Copy link
Collaborator Author

wycccccc commented Dec 3, 2022

添加 #1114 後續討論中 增加用戶確認blank line是否合法的選項
目前的blank line只討論最後一行爲blank line是否合理

另外修改了一下架構，將build移入了builder。確保其private，讓外界有builder以外的手段建構有些怪怪的。

@wycccccc wycccccc changed the title Add Import csv.md Add Import csv.md & blank line argument Dec 3, 2022
chia7712
chia7712 previously approved these changes Dec 3, 2022
View reviewed changes
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

LGTM 兩個小地方修正完就可以合併了

@wycccccc wycccccc merged commit 94134b4 into opensource4you:main Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chinghongfang chinghongfang chinghongfang approved these changes

@chia7712 chia7712 chia7712 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Astraea tool : Import csv
3 participants
@wycccccc @chia7712 @chinghongfang