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

Have last argument be the instance name to match other koha-* scripts? #12

Open
wizzyrea opened this issue Apr 8, 2015 · 6 comments
Open
Assignees

Comments

@wizzyrea
Copy link
Contributor

wizzyrea commented Apr 8, 2015

All of the other koha package scripts take the instance name as the last argument. This trips me up every time I use gitify. It's arbitrary but... would it be better to change it to be consistent with the other scripts?

(Yes, I am actually happy to submit a patch for this one if you think it's a good idea, I won't bother otherwise ^.^)

@chrisosaurus chrisosaurus self-assigned this Apr 9, 2015
@chrisosaurus
Copy link
Owner

Good point, I will grab this.

I want to make gitify support both forms, without having to specify flags,
so both gitify instance path and gitify path instance

gitify can tell them apart by checking if both exist as koha instances and if both are valid paths.

@wizzyrea
Copy link
Contributor Author

wizzyrea commented Apr 9, 2015

approve of this plan, that way it won't mess with people who are not as backwards as me :)

chrisosaurus pushed a commit that referenced this issue Apr 15, 2015
@chrisosaurus
Copy link
Owner

@wizzyrea I have pushed a potential (and untested) fix to a new branch 'smarter-argument-handling'

you should now be able to use both forms
koha-gitify instance git
koha-gitify git instance

If you could test and let me know what you think.

Sadly I do not have a koha instance I can currently test against.

@wizzyrea
Copy link
Contributor Author

Hi,

I tried this, and it seemed to work ok until the end - I got an error message as follows:

ERROR: from file '/etc/koha/sites//path/to/git/clone/koha-conf.xml' not found at ./koha-gitify line 91.
(/path/to/git/clone/ isn't literal of course, but it was the path to an existing and working git clone)
this was with the clone path first and the instance 2nd.

Conveniently, I made a typo the first time I ran it, and it did error out when the git path didn't exist properly with the instance on the end. So that was good. :)

@chrisosaurus
Copy link
Owner

@wizzyrea thanks for that, I had a silly bug in my handling of which argument was which (it would always get them backwards, which explains why you saw your git path appended to the end of /etc/koha/sites)

I have pushed a fix for this, I also added in a DIE statement that will stop it before it does any actual work so it won't do any more damage.

Thankfully it shouldn't have been able to make any changes to your actual koha instances sites /path/to/got/clone/koha-conf.xml is not a valid path.

Please pull the branch smarter-argument-handling and try again

My testing:
I have a git instance 'foo' and my git repo is in /home/chris/devel/koha

Success cases:

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify foo /home/chris/devel/koha
gitifying foo (/etc/koha/sites/foo) to point at '/home/chris/devel/koha'
STOPPING: not actually performing work at ./koha-gitify line 65.

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify /home/chris/devel/koha foo
gitifying foo (/etc/koha/sites/foo) to point at '/home/chris/devel/koha'
STOPPING: not actually performing work at ./koha-gitify line 65.

notice that in both of the above we have the line
gitifying foo (/etc/koha/sites/foo) to point at '/home/chris/devel/koha'
this is correct as it has detected foo as the instance and /home/chris/devel/koha as the git path in both cases.

error cases:

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify /home/chris/devel/koha fo
ERROR: instances not found at either '/home/chris/devel/koha' or 'fo'

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify /home/chris/devel/koh foo
ERROR: git checkouts not found at either '/home/chris/devel/koh' or 'foo'

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify foo /home/chris/devel/koh
ERROR: git checkouts not found at either 'foo' or '/home/chris/devel/koh'

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify foa /home/chris/devel/koha
ERROR: instances not found at either 'foa' or '/home/chris/devel/koha'

This looks good to me.

@chrisosaurus
Copy link
Owner

Pushed small change to make error messages correctly singular

testing

success:

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify foo /home/chris/devel/koha
gitifying foo (/etc/koha/sites/foo) to point at '/home/chris/devel/koha'
STOPPING: not actually performing work at ./koha-gitify line 65.

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify /home/chris/devel/koha foo
gitifying foo (/etc/koha/sites/foo) to point at '/home/chris/devel/koha'
STOPPING: not actually performing work at ./koha-gitify line 65.

error:

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify foo /home/chris/devel/koh
ERROR: git checkout not found at either 'foo' or '/home/chris/devel/koh'

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify fo /home/chris/devel/koha
ERROR: instance not found at either 'fo' or '/home/chris/devel/koha'

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify /home/chris/devel/koha fo
ERROR: instance not found at either '/home/chris/devel/koha' or 'fo'

chris@heimdall:~/devel/koha-gitify$ ./koha-gitify /home/chris/devel/koh foo
ERROR: git checkout not found at either '/home/chris/devel/koh' or 'foo'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants