Skip to content

Conversation

@gongweibao
Copy link
Collaborator

@gongweibao gongweibao commented Jun 12, 2017

Fix #156
Add test case of ls chunkmeta cp.

Copy link
Collaborator

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

  • Must have an issue for this PR
  • These test cases do not cover all the modules.
cat > ~/.paddle/config << EOF
datacenters:
- name: datacenter1
username: gongweibao@baidu.com
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not use actual usernames.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,10 @@
#/bin/bash
mkdir -p ~/.paddle
Copy link
Collaborator

@helinwang helinwang Jun 21, 2017

Choose a reason for hiding this comment

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

Maybe instead of generating this file with a script (unit test breaks without running this script), you can check in this file into the code base, maybe under testdata, like:https://github.com/golang/go/blob/master/src/net/testdata/hosts . And add argument to override the default location ~/.paddle to the test file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

config是一个全局变量,这个参数不太好覆盖。如果不用这个全局变量,需要改动的地方太多。这个能否后续再想想如何更简单的解决这问题?

@gongweibao gongweibao closed this Jun 23, 2017
@gongweibao gongweibao reopened this Jun 23, 2017
Copy link
Collaborator

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

LGTM

@gongweibao gongweibao merged commit 6ca58a3 into PaddlePaddle:develop Jun 23, 2017
@gongweibao gongweibao deleted the addtestcase branch August 21, 2017 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants