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

lolcommits plugin for protonet #251

Merged
merged 7 commits into from
Feb 14, 2015

Conversation

salzig
Copy link
Contributor

@salzig salzig commented Feb 3, 2015

This Plugin allows you to push the newly taken lolcommit to your protonet box.

You should follow the API Documentation found on your box under Help/"Protonet REST API" to get a api_token.

Have fun.

@dudemeister
Copy link

just saw this on our protonet box! +1 this is awesome!

@salzig
Copy link
Contributor Author

salzig commented Feb 8, 2015

rebased onto master

@salzig
Copy link
Contributor Author

salzig commented Feb 8, 2015

would love to integrate the git branch into the message. maybe we can merge #252 first.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.31%) to 38.9% when pulling 43c1149 on Salzig:plugin_lolprotonet into 0c79a8f on mroth:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.31%) to 38.9% when pulling 43c1149 on Salzig:plugin_lolprotonet into 0c79a8f on mroth:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.31%) to 38.9% when pulling 43c1149 on Salzig:plugin_lolprotonet into 0c79a8f on mroth:master.

@matthutchinson
Copy link
Member

Not sure what Protonet is, but this looks OK to me.

@mroth, I'm beginning to wonder with all the plugins we have that do Rest type requests with different headers, params or auth, whether one Rest plugin to rule them all would suffice. Maybe doing something clever where multiple endpoints could be specified in it's config (each with url, params, headers), and on the plugin's post commit, all requests would executed in parallel using something like Typhoeus/Faraday. Something to attempt when we get to the grand plugin extraction work 😄

@mroth
Copy link
Member

mroth commented Feb 9, 2015

@matthutchinson yeah, it might be good to standardize that, but then again its something we can also just leave up to to the plugins and let them specify dependencies in their own Gemfiles. I suspect most people using an upload plugin only use one.

I think there is an interesting question as to whether to merge this to the master codebase -- it's great work, but perhaps you and I should finally tackle #99 and this can be independently managed as a Gem as we start moving plugins out of main. And since #250 has landed in master now there is a workaround until we finish it up that allows people to use this.

@matthutchinson, since @salzig has a handle on how the code works maybe now is a good time for us to finally rip off that bandaid off and implement dynamic gem plugin loading, and have a developer make our first plugin for it. What do you think? We could even pair on it remotely. 😁

@salzig
Copy link
Contributor Author

salzig commented Feb 9, 2015

rebased onto master + add branch to message.

@salzig
Copy link
Contributor Author

salzig commented Feb 9, 2015

@mroth would love to see this merged and released.

IMHO: Plugins like this could in future be simple executables that get an json with all relevant informations as invocation parameter. Maybe lolcommits could use something like git-hooks with pre-store (before image is placed into ~/.lolcommits/$repo, for color/text/whatever manipulation) and post-store for different upload plugins.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 39.01% when pulling 741f00b on Salzig:plugin_lolprotonet into 10c2ec9 on mroth:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 39.01% when pulling 741f00b on Salzig:plugin_lolprotonet into 10c2ec9 on mroth:master.

@matthutchinson
Copy link
Member

@mroth for this one i'd say merge.

While it does come at a good time, we did have an older PR/plugin (now closed I think) that actually needed the externalizing work to be done. It required a gem that was only compatible with Ruby 1.9 and to me, that was the most compelling example to use, since it illustrated how plugins would no longer be tied to the base requirements of the lolcommits gem.

After merging this one, we could put a freeze on plugin submissions until we do the real work to extract... Your call tho..

I have responded on #99 too..

@mroth
Copy link
Member

mroth commented Feb 10, 2015

@matthutchinson I think we should hold off and try to do the work on #99, since the goal is to move all plugins out of the main codebase, merging now would just move something else in that we have to move back out (hopefully soon!).

(And minor note, but since it will be a new repository if we do it ourselves then the original author won't have credit on the commits.)

@salzig, if you take a look at #99 I updated recently with thoughts on how it could work -- basically you will be putting this plugin in its own repository that you control with a .gemspec, so that you can update it whenever you want and control versioning/dependencies. We would them bless it by forking the repository into a lolcommits organization on GitHub which you would be granted commit rights on. The goal is to enable plugin developers to move faster, and create more visibility/flexibility in the lolcommits community.

@salzig
Copy link
Contributor Author

salzig commented Feb 10, 2015

@mroth nice thing for the future, but i would prefer to tell my colleges and some friends to gem install lolcommits right now, instead of distributing a self build gem and tell them to install that.

I will happily be one of the first to use the plugin-api when it's done.

@mroth
Copy link
Member

mroth commented Feb 12, 2015

Okay, given the amount of refactoring we have to do, I'm fine with this being merged for now, but I'm hoping we can extract to a real plugin very soon. 🌠

@matthutchinson
Copy link
Member

Ok I agree too - i'll merge this one now, hopefully it doesn't screw up your other PRs. Perhaps we can do a point release to get this plugin in the wild (along with some of your initial refactoring).

matthutchinson added a commit that referenced this pull request Feb 14, 2015
@matthutchinson matthutchinson merged commit 1f374ed into lolcommits:master Feb 14, 2015
@matthutchinson
Copy link
Member

Hi @salzig 👋

So you may (or may not) have noticed that we recently extracted all lolcommit plugins to external gems!

🏅 Since you contributed this protonet plugin, I've added you (as an admin) to the new lolcommits-protonet repo.

Of course you are under no obligation to maintain this code or gem going forward. This is just a quick message to explain what has happened and give you the opportunity to take ownership again.

If you're interested in picking this up, send me your email address (to matt/at/hiddeloop.com) and I'll add you as a new gem owner (with publishing rights) here.

Otherwise I will continue to maintain the gem, and make sure things are kept up to date.


To get an idea of how gem plugins work, check out the sample_plugin README / RubyDocs or browse through other plugin code.

The main reasons for extraction were to:

  • remove the ever-growing gem dependency list in the main lolcommits gem
  • start to unify configuration code for plugins (common helpers, approaches to OAuth etc.)
  • make lolcommit plugin development easier
  • add more comprehensive plugin tests

That's it, any questions just let me know.

Happy coding!

Matt

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.

None yet

5 participants