Closed ryantm closed 8 years ago
Thanks Ryan. It's all cosmetic except for the 8add24a commit. What OS and Python version have you tested it on?
I'm not sure about removing the specialization of Daemon (MyDaemon). I believe Daemon was lifted from somewhere (not sure where) and is totally general purpose. Putting the runEyeFi() call in Daemon makes Daemon specific to this project. But I agree that the fact that Daemon requires someone to subclass it in order to use is silly.
I would support changing Daemon to take an argument on the init method that is a function pointer (the function to "daemonize"), that will be called in the run method, or something like that....
Daemon should probably be in its own module to make it perfectly clear that it is sort of a third-party dependency (don't worry about that, just sayin')
Thanks for the feedback! I will try to consider it as I work on this branch more. I wouldn't recommend merging this yet. I've tested it on NixOS and Python 2.7.9. I'm also planning to make a NixOS expression for packaging this later unless someone else beats me to it.
On specializing the Daemon, my general feeling is that if this project is only going to have one file, we should avoid writing general code that makes the code longer. If it makes it shorter, I'm all for it!
Would be nice if we actually had proper upstream tagged and released tarballs...
Dave
On Mon, Jan 26, 2015 at 4:37 PM, Ryan Mulligan notifications@github.com wrote:
Thanks for the feedback! I will try to consider it as I work on this branch more. I wouldn't recommend merging this yet. I've tested it on NixOS and Python 2.7.9. I'm also planning to make a NixOS expression for packaging this later unless someone else beats me to it.
— Reply to this email directly or view it on GitHub https://github.com/dgrant/eyefiserver2/pull/17#issuecomment-71568699.
David Grant http://www.davidgrant.ca
I'm not sure what "upstream tagged" means. Your repo was the most upstream I found :smile:. :+1: for releases.
Well yeah we are the upstream, but we have no releases, no tags, no tarballs. Just a source code repository with a master branch and some feature branches. Actually we do have some tagging for the qnap releases I think (I don't deal with that side of things, someone else does that). But we have no releases or tarballs (besides a few qnap .qpkg releases).
David
On Mon, Jan 26, 2015 at 4:47 PM, Ryan Mulligan notifications@github.com wrote:
I'm not sure what "upstream tagged" means. Your repo was the most upstream I found [image: :smile:]. [image: :+1:] for releases.
— Reply to this email directly or view it on GitHub https://github.com/dgrant/eyefiserver2/pull/17#issuecomment-71569871.
David Grant http://www.davidgrant.ca
Yeah sorry my other email was confusing...
On Mon, Jan 26, 2015 at 4:50 PM, David Grant davidgrant@gmail.com wrote:
Well yeah we are the upstream, but we have no releases, no tags, no tarballs. Just a source code repository with a master branch and some feature branches. Actually we do have some tagging for the qnap releases I think (I don't deal with that side of things, someone else does that). But we have no releases or tarballs (besides a few qnap .qpkg releases).
David
On Mon, Jan 26, 2015 at 4:47 PM, Ryan Mulligan notifications@github.com wrote:
I'm not sure what "upstream tagged" means. Your repo was the most upstream I found [image: :smile:]. [image: :+1:] for releases.
— Reply to this email directly or view it on GitHub https://github.com/dgrant/eyefiserver2/pull/17#issuecomment-71569871.
David Grant http://www.davidgrant.ca
David Grant http://www.davidgrant.ca
Hi.
I'm not planning to work on this anymore, so I'm going to close it. I went ahead and wrote my own Eye-Fi server using Haskell: https://github.com/ryantm/heyefi
Ryan
I am starting a branch to clean up this code. Up to this point, it's mostly cosmetic.