Skip to content

Cmd line "application"#2

Merged
throughnothing merged 5 commits intothroughnothing:masterfrom
omega:cmd-line
Mar 27, 2013
Merged

Cmd line "application"#2
throughnothing merged 5 commits intothroughnothing:masterfrom
omega:cmd-line

Conversation

@omega
Copy link
Contributor

@omega omega commented Mar 26, 2013

Hey

This is by no means finished, I just figured I'd get some feedback before spending more time :) It fixes my needs for now, but wanted to know if something like this had a potential of being merged or not before spending more time

omega added 2 commits March 26, 2013 13:56
this script is pretty simple right now, with not documentation. It serves my current need :)
@throughnothing
Copy link
Owner

Hey, thanks for the PR! I know this repo needs a lot of work, and I actually stopped using pocket for now since it can't send to my kindle easily :( Hopefully they add that back, because I like pocket better. Anyway, this stuff looks good, and I'd definitely merge something like this :)

Let me know if you want me to review it the way it is, or if you do some more to it and then have me look over it later.

@omega
Copy link
Contributor Author

omega commented Mar 26, 2013

Hey

Let me add some docs then, and you can decide if that is enough for a "minimum viable product" to be merged :)

Out of curiosity, what are you using instead?

On Mar 26, 2013, at 2:30 PM, William Wolf notifications@github.com wrote:

Hey, thanks for the PR! I know this repo needs a lot of work, and I actually stopped using pocket for now since it can't send to my kindle easily :( Hopefully they add that back, because I like pocket better. Anyway, this stuff looks good, and I'd definitely merge something like this :)

Let me know if you want me to review it the way it is, or if you do some more to it and then have me look over it later.


Reply to this email directly or view it on GitHub.

@throughnothing
Copy link
Owner

Currently i'm using Readability. It has its own issues that I don't like (and doesn't support videos nicely like Pocket does), but it sends a daily digest to my kindle every day :) If Pocket makes it easy to send to kindle, I'll switch back, I like their interface + app the best.

@throughnothing
Copy link
Owner

Be sure and add yourself as an author to the dist.ini, too (if you want, of course).

omega added 2 commits March 26, 2013 17:07
This takes the form mainly of a small blurb in the Script.pm, and hiding attributes from --help.

Also satisfy some dzil warnings
This will add Contributors to all generated documentation, from git history.
@omega
Copy link
Contributor Author

omega commented Mar 26, 2013

Ok, pushed some changes.

I decided to not add myself as an author, but rather add the ContributersFromGit plugin, along with a basic weaver.ini. This is in a separate commit, so can be removed if you prefer the author solution instead :)

I added some command line help as well.

On Mar 26, 2013, at 2:52 PM, William Wolf notifications@github.com wrote:

Be sure and add yourself as an author to the dist.ini, too.


Reply to this email directly or view it on GitHub.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make these my ( $self ) = @_; to stay consistent with the rest of the code.

@throughnothing
Copy link
Owner

I like it, the last thing other than my 2 comments above, is can you keep all the lines to 80 width, again just to keep consistent with the way I've done everything else. Thanks!

@throughnothing
Copy link
Owner

Awesome, thanks for making those changes :) Looks good!

throughnothing added a commit that referenced this pull request Mar 27, 2013
@throughnothing throughnothing merged commit 9ffa881 into throughnothing:master Mar 27, 2013
@throughnothing
Copy link
Owner

Hmm, it seems like your weaver.ini has broken dzil build for me.

@throughnothing
Copy link
Owner

Also getting errors on print_usage_text:

The method 'print_usage_text' was not found in the inheritance hierarchy for WebService::Pocket::Script

@omega
Copy link
Contributor Author

omega commented Mar 27, 2013

ohh, sorry! maybe the weaver plugin is missing?

Pod::Weaver::Section::Contributors

Not sure how to add those, I'll check with the #distzilla crowd

On Mar 27, 2013, at 9:22 AM, William Wolf notifications@github.com wrote:

Hmm, it seems like your weaver.ini has broken dzil build for me.


Reply to this email directly or view it on GitHub.

@throughnothing
Copy link
Owner

I pushed some changes that temporarily get everything working for me, I want to release this to cpan once its sorted out correctly, so maybe another PR to clean that up, and then i'll release to cpan :)

@omega omega deleted the cmd-line branch March 27, 2013 02:43
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

Successfully merging this pull request may close these issues.

2 participants