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

Added tags to taglib to check for connected and disconnected state #20

Merged
merged 3 commits into from Mar 11, 2013

Conversation

Projects
None yet
2 participants
Contributor

ssherriff commented Feb 21, 2013

I've added in two tags, one to check if connected, and one to check if disconnected. I'm not 100% if I've done the check correctly, if I should just be checking for the token in the session to be null, but it should be easy to adjust if I haven't done it the right way. Let me know if something needs to be adjusted or not.

Basically, this just allows an easy way of in the gsp of showing certain content when connected and when disconnected.

Owner

antony commented Mar 3, 2013

Hey ssherriff,

Thanks for the contribution - great idea, I use similar tags myself but I hadn't gotten around to adding them to the plugin yet.

Unfortunately you didn't provide any tests for the tags you have written. I can't add code to the plugin without tests. Normally I would encourage writing tests first (TDD) but if you want to write a few tests to cover the new functionality and add them to this pull request that would be ok too.

Cheers for your contribution,
Antony

Contributor

ssherriff commented Mar 9, 2013

Hi Antony, sorry for the delay, but I've now added tests for these tags. Hopefully they are okay, but if not, let me know where it still needs work.

Cheers,
Steph

antony added a commit that referenced this pull request Mar 11, 2013

Merge pull request #20 from trexart/master
Added tags to taglib to check for connected and disconnected state

@antony antony merged commit f5739f7 into antony:master Mar 11, 2013

Owner

antony commented Mar 11, 2013

Thanks, this is perfect!

The only change I'll make when I merge is to move the common top two lines of each test into the setup() method because of the repetition, but beyond that, it's exactly as I would have hoped.

I'll merge + release it asap.

Cheers,
Antony

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