Add tags feature, refactoring some code #5

Merged
merged 4 commits into from Oct 4, 2011

Conversation

Projects
None yet
2 participants
Contributor

lunserv commented Sep 21, 2011

Add tags(snippets):
GuestTag
UserTag
AuthenticatedTag
NotAuthenticatedTag
PrincipalTag
Add HasAnyRoles to Locs
refactoring code, add isAuthenticatedOrRemembered method.

Why have you re-factored this into method overloads? Method overloads are typically not that good and considering we have default params in scala its preferable to use them.

These all need better non "-Tag" names.

Owner

timperrett commented Sep 29, 2011

Hey,

Why do they all have "Tag" suffixed? We're not making JSP taglibs, and its not idiomatic with the other snippets i've already made. Can you revise that? I've also added a couple of other notes inline.

Thanks for contributing.

Contributor

lunserv commented Sep 30, 2011

Hi,
I just gave the names with "Tag" suffixed as in Apache Shiro docs http://shiro.apache.org/web.html#Web-taglibrary .

Owner

timperrett commented Sep 30, 2011

Yes I know, but its not idiomatic for the rest of this library, as it would mean that you would end up writing:

lift:somethingtag

Which to me, seems silly because clearly, it is a tag, you dont need to suffix the word. It just screams Java is all and I want any new snippets to match the existing ones for style.

Contributor

lunserv commented Oct 1, 2011

Yes, I change names as:
IsGuest
IsUser
IsAuthenticated
IsNotAuthenticated.

Owner

timperrett commented Oct 3, 2011

Thanks for changing those names. The only thing that needs fixing now is the case statements in shiro.scala... those names represent the element names used within the markup, so they also need "tag" removing from them to bring them into line with the class names. Once thats done i'll happily merge this. Thanks for contributing.

Contributor

lunserv commented Oct 3, 2011

yes, sorry. I fix it.

@timperrett timperrett added a commit that referenced this pull request Oct 4, 2011

@timperrett timperrett Merge pull request #5 from lunserv/add_Tags_Feature
Added a range of new tags to have parity with those available in the regular Java Shiro edition.
4b3b422

@timperrett timperrett merged commit 4b3b422 into timperrett:master Oct 4, 2011

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