Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

make qs-push-plugin check that the file has a 32/64 bit binary before up... #1061

Merged
merged 2 commits into from Oct 5, 2012

Conversation

Projects
None yet
3 participants
Owner

pjrobertson commented Aug 23, 2012

...loading. Fixes #1060

I just made #1060 for the stats, hehe ;-)

An interesting note for @tiennou
At the moment you're using Trollop::die for when something fails, but this breaks things if you push a list of files to the script

qs-push-plugin a.qsplugin b.qsplugin...

It looks like you've done the right thing at one point (using puts then next). Should this be done everywhere?
Just wanted your opinion, but it shouldn't stop this pull request from going ahead

P.S. Thanks to @skurfer for the logic behind this. originally written in Python ;-)

Owner

tiennou commented Aug 24, 2012

Heh ;-). My nit-picking sez "Rubyists don't like CamelCase", "Stray return in diff". Otherwise I don't see anything else ;-).

IIRC the Trollop::die thing is that I preferred to be warned early in case something went wrong. Since I was testing with like 200 plugins it was a mess if one of those failed. I guess you could wrap the main upload loop with abegin… catch… end block and store each failing exception and report them at the end for a quick fix.

Owner

pjrobertson commented Aug 31, 2012

@tiennou
Didn't know CamelCase was against the rules ;-) Can I leave it in? :D

As for the return, is that not acceptable? It's in a function, so we're just returning from the function, no?
I'm assuming you're referring to pjrobertson/Quicksilver@6ab48ec#L0R38

Owner

tiennou commented Aug 31, 2012

No problem ;-). The return I spoke about was a return character, sorry it wasn't clear. And no problem too ;-).

Owner

pjrobertson commented Aug 31, 2012

Great, thanks.

I'll leave the Trollop die stuff as it is for now. I don't know of any case where we'll be uploading multiple plugins at once anyway. The -c option (changes) wouldn't work for multiple plugins either.

Can we merge this?

skurfer added a commit that referenced this pull request Oct 5, 2012

Merge pull request #1061 from pjrobertson/i1060
make qs-psuh-plugin check that the file has a 32/64 bit binary before up...

@skurfer skurfer merged commit 686b171 into quicksilver:master Oct 5, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment