GitHub PullRequestReviewCommentEvent at 2014-11-20 22:58:44
Repository: workshop-template
Actor: lzamparo
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/comments/20684767","id":20684767,"diff_hunk":"@@ -43,28 +43,31 @@\n ##   output to /dev/null, so that it runs quietly.  For debugging, you\n ##   may wish to change one or both of these.\n \n+\n # Check that the current working directory is *not* already a Git repo.\n function check_pwd {\n     success=$(git status > /dev/null 2>&1; echo $?)\n-    if [[ ${success} -eq 0 ]];\n+    if test ${success} -eq 0\n     then\n         echo \"ERROR: you are already inside a Git repository.\"\n         exit 2\n     fi\n }\n \n+# Check that the workshop_id contains only lower case characters.\n function check_lowercase {\n-  check=$(echo ${WORKSHOP_ID} | grep [A-Z] > /dev/null 2>&1; echo $?)\n-  if [[ ${check} -eq 0 ]];\n+  echo ${WORKSHOP_ID} | grep [A-Z] > /dev/null 2>&1\n+  if test $? -ne 1\n   then\n       echo \"ERROR: workshop_id cannot contain uppercase letters\"\n       exit 2\n   fi\n }\n \n+# Check that the workshop_id contains a hyphenated date\n function check_hyphens {\n-  success=$(echo ${WORKSHOP_ID} | grep -v \"-\" > /dev/null 2>&1; echo $?)\n-  if [[ ${success} -eq 0 ]];\n+  echo ${WORKSHOP_ID} | grep -v \"-\" > /dev/null 2>&1\n+  if test $? -ne 1","path":"tools/create","position":33,"original_position":33,"commit_id":"28513cab4b18d4d0a20d7538c63ba9180cb4d3b0","original_commit_id":"28513cab4b18d4d0a20d7538c63ba9180cb4d3b0","user":{"login":"lzamparo","id":1569186,"avatar_url":"https://avatars.githubusercontent.com/u/1569186?v=3","gravatar_id":"","url":"https://api.github.com/users/lzamparo","html_url":"https://github.com/lzamparo","followers_url":"https://api.github.com/users/lzamparo/followers","following_url":"https://api.github.com/users/lzamparo/following{/other_user}","gists_url":"https://api.github.com/users/lzamparo/gists{/gist_id}","starred_url":"https://api.github.com/users/lzamparo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lzamparo/subscriptions","organizations_url":"https://api.github.com/users/lzamparo/orgs","repos_url":"https://api.github.com/users/lzamparo/repos","events_url":"https://api.github.com/users/lzamparo/events{/privacy}","received_events_url":"https://api.github.com/users/lzamparo/received_events","type":"User","site_admin":false},"body":"Depends on the behaviour of $?, which I think interacts with how you test the value.  When I run the following test case in test_create:\r\n\r\n```\r\n# Check that the workshop_id contains a hyphenated date\r\nfunction check_hyphens {\r\n  echo ${WORKSHOP_ID} | grep -v \"-\" > /dev/null 2>&1\r\n  if test $? -ne 1\r\n  then\r\n      echo \"ERROR: workshop_id must contain hyphens for the date.\"\r\n      kill -2 $$\r\n  fi\r\n}\r\n\r\nfunction test_check_hyphens {\r\n    trap '' 2\r\n    WORKSHOP_ID=\"20140608\"\r\n    echo \"check_hyphens test: $WORKSHOP_ID ...\"\r\n    check_hyphens\r\n    trap - 2\r\n    WORKSHOP_ID=\"2014-06-08\"\r\n    echo \"check_hyphens test: $WORKSHOP_ID  ...\"\r\n    check_hyphens\r\n}\r\n\r\n# Main driver.\r\nfunction tests {\r\n    test_check_hyphens\r\n}\r\n\r\ntests\r\n```\r\n\r\nI get an error when WORKSHOP_ID contains no hyphens, and no error with it does contain hyphens:\r\n\r\n```\r\nlee@beehive:~/projects/workshop-template/tools$ ./test_create \r\ncheck_hyphens test: 20140608 ...\r\nERROR: workshop_id must contain hyphens for the date.\r\ncheck_hyphens test: 2014-06-08  ...\r\n```\r\n\r\nIs this not the expected behaviour? \r\n\r\nAs an aside, I added comments to the functions, but they don't exactly match what the function does.  check_hyphens only checks for the presence of hyphens in WORKSHOP_ID, but the error message suggests WORKSHOP_ID also contain a valid (hyphenated) date in the string.  Should we be doing a more rigorous check here?","created_at":"2014-11-20T22:58:44Z","updated_at":"2014-11-20T22:58:44Z","html_url":"https://github.com/swcarpentry/workshop-template/pull/66#discussion_r20684767","pull_request_url":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66","_links":{"self":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/comments/20684767"},"html":{"href":"https://github.com/swcarpentry/workshop-template/pull/66#discussion_r20684767"},"pull_request":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66"}}},"pull_request":{"url":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66","id":24790922,"html_url":"https://github.com/swcarpentry/workshop-template/pull/66","diff_url":"https://github.com/swcarpentry/workshop-template/pull/66.diff","patch_url":"https://github.com/swcarpentry/workshop-template/pull/66.patch","issue_url":"https://api.github.com/repos/swcarpentry/workshop-template/issues/66","number":66,"state":"closed","locked":false,"title":"standardized if test syntax for bash functions","user":{"login":"lzamparo","id":1569186,"avatar_url":"https://avatars.githubusercontent.com/u/1569186?v=3","gravatar_id":"","url":"https://api.github.com/users/lzamparo","html_url":"https://github.com/lzamparo","followers_url":"https://api.github.com/users/lzamparo/followers","following_url":"https://api.github.com/users/lzamparo/following{/other_user}","gists_url":"https://api.github.com/users/lzamparo/gists{/gist_id}","starred_url":"https://api.github.com/users/lzamparo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lzamparo/subscriptions","organizations_url":"https://api.github.com/users/lzamparo/orgs","repos_url":"https://api.github.com/users/lzamparo/repos","events_url":"https://api.github.com/users/lzamparo/events{/privacy}","received_events_url":"https://api.github.com/users/lzamparo/received_events","type":"User","site_admin":false},"body":"WRT #65 \r\n\r\nI've standardized the syntax used to test return codes for the if statements in helper functions. ","created_at":"2014-11-20T19:28:41Z","updated_at":"2014-11-20T22:58:44Z","closed_at":"2014-11-20T22:07:07Z","merged_at":"2014-11-20T22:07:07Z","merge_commit_sha":"ae0f7c6a30d2ba6cb73745f75d756683c91f4b63","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66/commits","review_comments_url":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66/comments","review_comment_url":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/comments/{number}","comments_url":"https://api.github.com/repos/swcarpentry/workshop-template/issues/66/comments","statuses_url":"https://api.github.com/repos/swcarpentry/workshop-template/statuses/28513cab4b18d4d0a20d7538c63ba9180cb4d3b0","head":{"label":"lzamparo:standardize_create","ref":"standardize_create","sha":"28513cab4b18d4d0a20d7538c63ba9180cb4d3b0","user":{"login":"lzamparo","id":1569186,"avatar_url":"https://avatars.githubusercontent.com/u/1569186?v=3","gravatar_id":"","url":"https://api.github.com/users/lzamparo","html_url":"https://github.com/lzamparo","followers_url":"https://api.github.com/users/lzamparo/followers","following_url":"https://api.github.com/users/lzamparo/following{/other_user}","gists_url":"https://api.github.com/users/lzamparo/gists{/gist_id}","starred_url":"https://api.github.com/users/lzamparo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lzamparo/subscriptions","organizations_url":"https://api.github.com/users/lzamparo/orgs","repos_url":"https://api.github.com/users/lzamparo/repos","events_url":"https://api.github.com/users/lzamparo/events{/privacy}","received_events_url":"https://api.github.com/users/lzamparo/received_events","type":"User","site_admin":false},"repo":{"id":26506761,"name":"workshop-template","full_name":"lzamparo/workshop-template","owner":{"login":"lzamparo","id":1569186,"avatar_url":"https://avatars.githubusercontent.com/u/1569186?v=3","gravatar_id":"","url":"https://api.github.com/users/lzamparo","html_url":"https://github.com/lzamparo","followers_url":"https://api.github.com/users/lzamparo/followers","following_url":"https://api.github.com/users/lzamparo/following{/other_user}","gists_url":"https://api.github.com/users/lzamparo/gists{/gist_id}","starred_url":"https://api.github.com/users/lzamparo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lzamparo/subscriptions","organizations_url":"https://api.github.com/users/lzamparo/orgs","repos_url":"https://api.github.com/users/lzamparo/repos","events_url":"https://api.github.com/users/lzamparo/events{/privacy}","received_events_url":"https://api.github.com/users/lzamparo/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/lzamparo/workshop-template","description":"Template for workshop websites.","fork":true,"url":"https://api.github.com/repos/lzamparo/workshop-template","forks_url":"https://api.github.com/repos/lzamparo/workshop-template/forks","keys_url":"https://api.github.com/repos/lzamparo/workshop-template/keys{/key_id}","collaborators_url":"https://api.github.com/repos/lzamparo/workshop-template/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/lzamparo/workshop-template/teams","hooks_url":"https://api.github.com/repos/lzamparo/workshop-template/hooks","issue_events_url":"https://api.github.com/repos/lzamparo/workshop-template/issues/events{/number}","events_url":"https://api.github.com/repos/lzamparo/workshop-template/events","assignees_url":"https://api.github.com/repos/lzamparo/workshop-template/assignees{/user}","branches_url":"https://api.github.com/repos/lzamparo/workshop-template/branches{/branch}","tags_url":"https://api.github.com/repos/lzamparo/workshop-template/tags","blobs_url":"https://api.github.com/repos/lzamparo/workshop-template/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/lzamparo/workshop-template/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/lzamparo/workshop-template/git/refs{/sha}","trees_url":"https://api.github.com/repos/lzamparo/workshop-template/git/trees{/sha}","statuses_url":"https://api.github.com/repos/lzamparo/workshop-template/statuses/{sha}","languages_url":"https://api.github.com/repos/lzamparo/workshop-template/languages","stargazers_url":"https://api.github.com/repos/lzamparo/workshop-template/stargazers","contributors_url":"https://api.github.com/repos/lzamparo/workshop-template/contributors","subscribers_url":"https://api.github.com/repos/lzamparo/workshop-template/subscribers","subscription_url":"https://api.github.com/repos/lzamparo/workshop-template/subscription","commits_url":"https://api.github.com/repos/lzamparo/workshop-template/commits{/sha}","git_commits_url":"https://api.github.com/repos/lzamparo/workshop-template/git/commits{/sha}","comments_url":"https://api.github.com/repos/lzamparo/workshop-template/comments{/number}","issue_comment_url":"https://api.github.com/repos/lzamparo/workshop-template/issues/comments/{number}","contents_url":"https://api.github.com/repos/lzamparo/workshop-template/contents/{+path}","compare_url":"https://api.github.com/repos/lzamparo/workshop-template/compare/{base}...{head}","merges_url":"https://api.github.com/repos/lzamparo/workshop-template/merges","archive_url":"https://api.github.com/repos/lzamparo/workshop-template/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/lzamparo/workshop-template/downloads","issues_url":"https://api.github.com/repos/lzamparo/workshop-template/issues{/number}","pulls_url":"https://api.github.com/repos/lzamparo/workshop-template/pulls{/number}","milestones_url":"https://api.github.com/repos/lzamparo/workshop-template/milestones{/number}","notifications_url":"https://api.github.com/repos/lzamparo/workshop-template/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/lzamparo/workshop-template/labels{/name}","releases_url":"https://api.github.com/repos/lzamparo/workshop-template/releases{/id}","created_at":"2014-11-11T21:58:03Z","updated_at":"2014-11-08T15:54:11Z","pushed_at":"2014-11-20T19:27:24Z","git_url":"git://github.com/lzamparo/workshop-template.git","ssh_url":"git@github.com:lzamparo/workshop-template.git","clone_url":"https://github.com/lzamparo/workshop-template.git","svn_url":"https://github.com/lzamparo/workshop-template","homepage":null,"size":105,"stargazers_count":0,"watchers_count":0,"language":null,"has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"gh-pages"}},"base":{"label":"swcarpentry:gh-pages","ref":"gh-pages","sha":"8155e3334512bea16ee18607f6a3a8997f363294","user":{"login":"swcarpentry","id":2238358,"avatar_url":"https://avatars.githubusercontent.com/u/2238358?v=3","gravatar_id":"","url":"https://api.github.com/users/swcarpentry","html_url":"https://github.com/swcarpentry","followers_url":"https://api.github.com/users/swcarpentry/followers","following_url":"https://api.github.com/users/swcarpentry/following{/other_user}","gists_url":"https://api.github.com/users/swcarpentry/gists{/gist_id}","starred_url":"https://api.github.com/users/swcarpentry/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swcarpentry/subscriptions","organizations_url":"https://api.github.com/users/swcarpentry/orgs","repos_url":"https://api.github.com/users/swcarpentry/repos","events_url":"https://api.github.com/users/swcarpentry/events{/privacy}","received_events_url":"https://api.github.com/users/swcarpentry/received_events","type":"Organization","site_admin":false},"repo":{"id":25652980,"name":"workshop-template","full_name":"swcarpentry/workshop-template","owner":{"login":"swcarpentry","id":2238358,"avatar_url":"https://avatars.githubusercontent.com/u/2238358?v=3","gravatar_id":"","url":"https://api.github.com/users/swcarpentry","html_url":"https://github.com/swcarpentry","followers_url":"https://api.github.com/users/swcarpentry/followers","following_url":"https://api.github.com/users/swcarpentry/following{/other_user}","gists_url":"https://api.github.com/users/swcarpentry/gists{/gist_id}","starred_url":"https://api.github.com/users/swcarpentry/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swcarpentry/subscriptions","organizations_url":"https://api.github.com/users/swcarpentry/orgs","repos_url":"https://api.github.com/users/swcarpentry/repos","events_url":"https://api.github.com/users/swcarpentry/events{/privacy}","received_events_url":"https://api.github.com/users/swcarpentry/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/swcarpentry/workshop-template","description":"Template for workshop websites.","fork":false,"url":"https://api.github.com/repos/swcarpentry/workshop-template","forks_url":"https://api.github.com/repos/swcarpentry/workshop-template/forks","keys_url":"https://api.github.com/repos/swcarpentry/workshop-template/keys{/key_id}","collaborators_url":"https://api.github.com/repos/swcarpentry/workshop-template/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/swcarpentry/workshop-template/teams","hooks_url":"https://api.github.com/repos/swcarpentry/workshop-template/hooks","issue_events_url":"https://api.github.com/repos/swcarpentry/workshop-template/issues/events{/number}","events_url":"https://api.github.com/repos/swcarpentry/workshop-template/events","assignees_url":"https://api.github.com/repos/swcarpentry/workshop-template/assignees{/user}","branches_url":"https://api.github.com/repos/swcarpentry/workshop-template/branches{/branch}","tags_url":"https://api.github.com/repos/swcarpentry/workshop-template/tags","blobs_url":"https://api.github.com/repos/swcarpentry/workshop-template/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/swcarpentry/workshop-template/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/swcarpentry/workshop-template/git/refs{/sha}","trees_url":"https://api.github.com/repos/swcarpentry/workshop-template/git/trees{/sha}","statuses_url":"https://api.github.com/repos/swcarpentry/workshop-template/statuses/{sha}","languages_url":"https://api.github.com/repos/swcarpentry/workshop-template/languages","stargazers_url":"https://api.github.com/repos/swcarpentry/workshop-template/stargazers","contributors_url":"https://api.github.com/repos/swcarpentry/workshop-template/contributors","subscribers_url":"https://api.github.com/repos/swcarpentry/workshop-template/subscribers","subscription_url":"https://api.github.com/repos/swcarpentry/workshop-template/subscription","commits_url":"https://api.github.com/repos/swcarpentry/workshop-template/commits{/sha}","git_commits_url":"https://api.github.com/repos/swcarpentry/workshop-template/git/commits{/sha}","comments_url":"https://api.github.com/repos/swcarpentry/workshop-template/comments{/number}","issue_comment_url":"https://api.github.com/repos/swcarpentry/workshop-template/issues/comments/{number}","contents_url":"https://api.github.com/repos/swcarpentry/workshop-template/contents/{+path}","compare_url":"https://api.github.com/repos/swcarpentry/workshop-template/compare/{base}...{head}","merges_url":"https://api.github.com/repos/swcarpentry/workshop-template/merges","archive_url":"https://api.github.com/repos/swcarpentry/workshop-template/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/swcarpentry/workshop-template/downloads","issues_url":"https://api.github.com/repos/swcarpentry/workshop-template/issues{/number}","pulls_url":"https://api.github.com/repos/swcarpentry/workshop-template/pulls{/number}","milestones_url":"https://api.github.com/repos/swcarpentry/workshop-template/milestones{/number}","notifications_url":"https://api.github.com/repos/swcarpentry/workshop-template/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/swcarpentry/workshop-template/labels{/name}","releases_url":"https://api.github.com/repos/swcarpentry/workshop-template/releases{/id}","created_at":"2014-10-23T18:57:40Z","updated_at":"2014-11-12T09:49:07Z","pushed_at":"2014-11-20T22:07:07Z","git_url":"git://github.com/swcarpentry/workshop-template.git","ssh_url":"git@github.com:swcarpentry/workshop-template.git","clone_url":"https://github.com/swcarpentry/workshop-template.git","svn_url":"https://github.com/swcarpentry/workshop-template","homepage":null,"size":589,"stargazers_count":1,"watchers_count":1,"language":null,"has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":16,"mirror_url":null,"open_issues_count":10,"forks":16,"open_issues":10,"watchers":1,"default_branch":"gh-pages"}},"_links":{"self":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66"},"html":{"href":"https://github.com/swcarpentry/workshop-template/pull/66"},"issue":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/issues/66"},"comments":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/issues/66/comments"},"review_comments":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66/comments"},"review_comment":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/pulls/66/commits"},"statuses":{"href":"https://api.github.com/repos/swcarpentry/workshop-template/statuses/28513cab4b18d4d0a20d7538c63ba9180cb4d3b0"}}}}