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

Yammer plugin #160

Merged
merged 20 commits into from
Apr 12, 2014
Merged

Yammer plugin #160

merged 20 commits into from
Apr 12, 2014

Conversation

mrclmrvn
Copy link
Contributor

Allows to post lolcommits image on organization's yammer account

@mrclmrvn
Copy link
Contributor Author

Instructions print during run 👎

@OscarGodson
Copy link

+1000

@mrclmrvn
Copy link
Contributor Author

Moved the yammer setup instructions in post_install_message, until there's a better way to do this during configuration.

@OscarGodson
Copy link

No explanation why this was closed?

@matthutchinson
Copy link
Member

apologies, for some reason a whole bunch of PRs got closed at once, I'm attempting to reopen now, but GitHub's site status is up/down for me

@matthutchinson
Copy link
Member

No longer mergable..

We've recently made changes to our plugin code, including a feature allowing each plugin to define its own configuration implementation (that executes when you run lolcommits --config --plugin plugin_name.

As an example, take a look at the latest twitter plugin code on master, particularly where it overrides the configure_plugin! method.

Please also look at the latest changes in the base plugin class that you might be able to use to hook up support for Yammer.

E.g. the stealth flag is honored by default when calling puts inside plugin classes.

@mrclmrvn
Copy link
Contributor Author

mrclmrvn commented Dec 4, 2013

Thanks @matthutchinson. I will look into configure_plugin! and use it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.04%) when pulling cf3e853 on mrclmrvn:master into 5c0de5c on mroth:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 65ed2cc on mrclmrvn:master into 5c0de5c on mroth:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.16%) when pulling afa10ab on mrclmrvn:master into 5c0de5c on mroth:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.16%) when pulling 190ee53 on mrclmrvn:master into 9d62c26 on mroth:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.16%) when pulling f92c3a2 on mrclmrvn:master into 9d62c26 on mroth:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.45%) when pulling 7856ffd on mrclmrvn:master into 9d62c26 on mroth:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7856ffd on mrclmrvn:master into 9d62c26 on mroth:master.

@mrclmrvn
Copy link
Contributor Author

mrclmrvn commented Jan 5, 2014

I have submitted for approval 3 days ago, so it can be globally available. No response yet.

Imgur

@mrclmrvn
Copy link
Contributor Author

mrclmrvn commented Jan 8, 2014

Approved for global use

Imgur

require 'yammer'
require 'rest_client'

# https://developer.yammer.com/oauth2-quickstart/
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these two constants (CLIENT_ID and CLIENT_SECRET) also be asked for (and stored) in the configuration? OR are these CLIENT constants merely to identify the lolcommits yammer plugin client to yammer itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matthutchinson

CLIENT_ID and CLIENT_SECRET is for the registered app in Yammer Application directory, so it's merely to identify the lolcommits yammer plugin client to yammer itself.

I'll lookup transferring ownership procedure of the app, if anyone wants to take it :)

Right now, I don't there's much to configure on it, unless someone wants it published in Yammer Application directory (maybe add proper description, screenshots, etc).

@matthutchinson
Copy link
Member

Looks good, just a couple of comments and a question on those constants, otherwise looks good to merge.

@matthutchinson
Copy link
Member

@mrclmrvn thanks for amending, and apologies for the delay, merging this now 👍

There will be a new gem release with this soon.

matthutchinson added a commit that referenced this pull request Apr 12, 2014
@matthutchinson matthutchinson merged commit d94f284 into lolcommits:master Apr 12, 2014
@mrclmrvn
Copy link
Contributor Author

👍 @matthutchinson

@matthutchinson
Copy link
Member

Hi @mrclmrvn 👋

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

🏅 Since you contributed this yammer plugin, I've added you (as an admin) to the new lolcommits-yammer 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

@mrclmrvn
Copy link
Contributor Author

👍

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

4 participants