Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

remove unused type attribute in Core::Role::Engine #149

Closed
wants to merge 1 commit into from

4 participants

@dagolden

The type attribute is unused, so I removed it.

@celogeek
Owner

This is not used now, but have a type could be usefull for plugins ?

Like Template plugins, or Logger plugin or Serializer plugin. And we can use that to do stuff.

Of course it seems useless now, but it could be helpfull for plugins.

@dagolden

But what does it mean and what is it used for? Until that is defined, it shouldn't be in the code base.

There's no limitation on what can go in there, so if plugins start populating it with who-knows-what, and then semantics are attached to it, things might break.

@celogeek
Owner

Well, better to have a clear and sufficiant code that code that may handle too much think and seems useless for now.

@celogeek
Owner

That patch define the SessionFactory type.

#150

@dagolden

That illustrates my point. SessionFactory is the class name, but the config is under "session" under the "engines" config option. Should 'type' be "Session" or "SessionFactory"? Who knows.

Nothing that uses engines cares about the 'type' defined by Core::Role::Engine, so there's no point to making consumers of the class define one.

@sukria
Owner

I agree with @dagolden : if something is not used in the code, it should not be there.

@celogeek : do you agree we can remove that unused stuff?

@sukria
Owner

@celogeek : ping :)

@yanick
Owner

Fwiw: +1 also on the removal.

@sukria
Owner

Done. Thanks.

@sukria sukria referenced this pull request from a commit
Alexis Sukrieh backport changes from PR #149 to remove 'type' b699ec2
@sukria sukria closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 20, 2013
  1. @dagolden
This page is out of date. Refresh to see the latest.
View
8 lib/Dancer/Core/Role/Engine.pm
@@ -6,12 +6,6 @@ use Dancer::Core::Types;
with 'Dancer::Core::Role::Hookable';
-has type => (
- is => 'ro',
- lazy => 1,
- builder => 1,
-);
-
has environment => (is => 'ro');
has location => (is => 'ro');
@@ -28,6 +22,4 @@ has config => (
default => sub { {} },
);
-requires '_build_type';
-
1;
View
2  lib/Dancer/Core/Role/Logger.pm
@@ -14,8 +14,6 @@ sub supported_hooks {
);
}
-sub _build_type {'Logger'}
-
# This is the only method to implement by logger engines.
# It receives the following arguments:
# $msg_level, $msg_content, it gets called only if the configuration allows
View
2  lib/Dancer/Core/Role/Serializer.pm
@@ -13,8 +13,6 @@ sub supported_hooks {
);
}
-sub _build_type {'Serializer'}
-
requires 'serialize';
requires 'deserialize';
requires 'loaded';
View
4 lib/Dancer/Core/Role/SessionFactory.pm
@@ -38,10 +38,6 @@ sub supported_hooks {
/;
}
-sub _build_type {
- 'SessionFactory';
-} # XXX vs 'Session'? Unused, so I can't tell -- xdg
-
=attr cookie_name
The name of the cookie to create for storing the session key
View
2  lib/Dancer/Core/Role/Template.pm
@@ -19,8 +19,6 @@ sub supported_hooks {
/;
}
-sub _build_type {'Template'}
-
requires 'render';
has name => (
View
1  t/engine.t
@@ -9,6 +9,5 @@ ok($f->does('Dancer::Core::Role::Engine'));
ok($f->does('Dancer::Core::Role::Template'));
is $f->name, 'Tiny';
-is $f->type, 'Template';
done_testing;
Something went wrong with that request. Please try again.