search

dspinellis / git-issue

Git-based decentralized issue management
GNU General Public License v3.0
773 stars 66 forks source link

Lower case command arguments user and repo #85

Closed BoniLindsley closed 4 years ago

BoniLindsley commented 4 years ago

Fix #84: Ensure import and export find the same issue directory.

I have added lower casing to import/export functions that parse command line arguments. I think I have caught all of them. Four considerations had come to mind:

1) The create_issue function is used internally in a for loop in two other functions. So this adds redundant lower casing in these loops. This is unlikely to be a bottleneck though. 2) I did not add a test case to test.sh. I can have a go at it, but it will take me a bit of time to understand the test script. 3) This commit is likely to clash with #82. The pull request in there may also need to consider letter cases? 4) I decided against lower casing $provider. All definitions of the variable from user input are preceded by an explicit comparison to github and gitlab, making the lower casing entirely redundant. Moving the comparison to after the lower casing is possible to allow, for example, git issue import GitLab <...>. Please let me know if that is desirable.

FrazerClews commented 4 years ago
  1. This commit is likely to clash with #82. The pull request in there may also need to consider letter cases?

Thanks for the heads up. Mine is ready for review for the functionality, but testing ideally needs implementing, but still going through how to implement it. I don't mind if #82 or #85 is merged first, I could add the tests in another patch if that's easier

As for if #82 will need to consider it, I don't think so since it fetches the information from git remote get-url ${remote}|origin. It may need considering for ${remote} and will need rebasing

dspinellis commented 4 years ago

Thank you!

dspinellis commented 4 years ago

Can you please fix the shellcheck warnings reported in the Travis CI build?