Commit | Line | Data |
01dfb279 |
1 | 15:36 <t0m> right. so the first thing to do is probably write a test to check |
2 | generating an application doesn't change. |
3 | 15:37 <dhoss> per app? |
4 | 15:38 <t0m> no, I mean whilst you're ripping ::Helper apart. |
5 | 15:38 <dhoss> OH |
6 | 15:38 <dhoss> gotcha |
7 | 15:38 <t0m> if you install your branch.. Then catalyst.pl t/TestAppForComparison |
8 | 15:39 <dhoss> so we have the same results now as before, yes? |
9 | 15:39 <t0m> then you write a test which says catalyst.pl /tmp/TempTestApp; my |
10 | $diff = `diff -urN t/TestAppForComparison /tmp/TempTestApp`; ok |
11 | !length($diff) or warn $diff; |
12 | 15:40 <t0m> exactly. and that'll test you rip all of the files out of Helper.pm |
13 | correctly.. |
14 | 15:40 <dhoss> okay, TestAppForComparison created. i'll commit that initial |
15 | version |
16 | 15:40 <t0m> you'll probably want to nuke it after that, as testing the |
17 | generated strings of an entire app dir is gonna be a PITA |
18 | 15:41 <t0m> to maintain |
19 | 15:41 <dhoss> i could see that |
20 | 15:41 <t0m> But it's totally the right solution whilst you're trying to rip |
21 | stuff out without changing it |
22 | 15:41 <dhoss> temp testing :-D |
23 | 15:44 <t0m> I've got some ideas on doing better testing that the TestApp we |
24 | generate is good. |
25 | 15:44 <t0m> but they're a bit more involved, and not needed for this at all ;) |
26 | 15:45 <dhoss> this being this refactor, or this bit of the refactor? |
27 | 15:45 <t0m> the latter. |
28 | 15:45 <dhoss> okay - got it |
29 | 15:46 <t0m> and once this is done - you can branch again to work on the code, |
30 | and again to work on the generated app templates (as we'll want to |
31 | moosify those, so your app is moosetastic by default |
32 | 15:46 <t0m> and as it's all in different files - you can do both in parallel as |
33 | you feel like and safely merge back |
34 | 15:47 <t0m> did you get to grips with MX::GetOpt and do you have a plan for |
35 | that yet? |
36 | 15:48 <dhoss> so one branch for helper_refactor, one for code (which code?), |
37 | and one for app templates |
38 | 15:48 <t0m> As I guess catalyst.pl should be taught to use it, and so should |
39 | the generated scripts :) |
40 | 15:48 <dhoss> i've been playing with MX::GetOpt, i need a few more details on |
41 | what exactly needs to be beat up |
42 | 15:48 <t0m> yy, but you branch from completed helper_refactor :) |
43 | 15:49 <t0m> and 'code' is the actual refactor / api changes for helpers |
44 | 15:49 <t0m> bletch |
45 | 15:50 <dhoss> okay, so the 'code' branch will be simply the C::H code branched |
46 | from helper_refactor |
47 | 15:50 <t0m> get_file has to keep working as everything using ::Helper needs it |
48 | 15:50 <t0m> yes, exactly. |
49 | 15:51 <t0m> so you need to make a get_file_from_sharedir function or something |
50 | 15:51 <dhoss> okay, making sense now :-) |
51 | 15:51 <t0m> and change things over to use that. |
52 | 15:51 <t0m> that's fine though - means you can extract 1 file at a time |
53 | 15:52 <dhoss> which would be better, no? better atomicity and for testing |
54 | 15:52 <t0m> yy |
55 | 15:52 <t0m> bletch was at the fact that 'get_file' is a good method name, and |
56 | it's now deprecated |
57 | 15:53 <dhoss> is that in C::H? |
58 | 15:53 <t0m> yy |
59 | 15:53 * dhoss is a little behind in my api knowledge |
60 | 15:53 <dhoss> off the top of my head at least |
61 | 15:53 <t0m> it's the 'get me a file out the <DATA> section' method |
62 | 15:53 <dhoss> aha right |
63 | 15:54 <t0m> which you're replacing in favour of 'get me a file out of the share |
64 | dir' |
65 | 15:54 <t0m> and 'copy file out of sharedir' methods. |
66 | 15:54 <t0m> The former for templates |
67 | 15:54 <t0m> the latter for binaries, which gets rid of the hateful decoding and |
68 | writing out business |
69 | 15:54 <dhoss> get -> we're going to interpolate shit, copy -> we're giong to |
70 | use directly? |
71 | 15:55 <t0m> just so |
72 | 15:55 <t0m> eliminating the 'please turn this giant hex string back into |
73 | binary' thing |
74 | 15:55 <dhoss> definitely :-P |
75 | 16:00 <t0m> sub _mk_favicon { |
76 | 16:00 <t0m> my ( $self ) = @_; |
77 | 16:00 <t0m> $self->copy_share_file( |
78 | 16:00 <t0m> File::Spec->catfile( ( caller(0) )[0], 'favicon.ico' ), |
79 | 16:00 <t0m> File::Spec->catfile( $self->{root}, "favicon.ico" ) |
80 | 16:00 <t0m> ); |
81 | 16:00 <t0m> } |
82 | 16:00 <t0m> sumfin like dat |
83 | 16:01 <dhoss> easy enough |
84 | 16:01 <t0m> the actually, all the caller crap dies too |
85 | 16:02 <dhoss> hrm, i suppose we have to keep the atrocious |
86 | Class::Accessor::Fast method name artifacts? |
87 | 16:02 <t0m> that's to clue get_file what file to borg crap out off |
88 | 16:03 <t0m> I vote that most of them can burn, as we'll be working out what |
89 | shit to copy a lot more dynamically |
90 | 16:03 <t0m> and the ones which can't get deprecated, but preserved. |
91 | 16:04 <dhoss> so that's just for the current get_file, right? |
92 | 16:04 <t0m> yy |
11dbddd9 |
93 | ######################################################################## |
94 | ## done |
95 | 16:04 <t0m> $self->copy_share_file( |
01dfb279 |
96 | 16:04 <t0m> 'favicon.ico', |
97 | 16:04 <t0m> File::Spec->catfile( $self->{root}, "favicon.ico" ) |
98 | 16:04 <t0m> ); |
11dbddd9 |
99 | ######################################################################### |
01dfb279 |
100 | 16:05 <t0m> more like it |
101 | 16:05 <t0m> so we have sub mk_foo { shift->generate_foo(@_); } |
102 | |
c5387711 |
103 | ==== |
104 | 5-28 |
5430247e |
105 | 03:04 < dhoss> SO. official next step: 1) #done trim crap out 2) #done write test for how |
c5387711 |
106 | helpers are invoking Catalyst::Helper 3)??? 4) profit! |
6be0f534 |
107 | 03:05 < t0m> 3)#done Re arrange files in sharedir so we can make the comversion guts |
c5387711 |
108 | totally generic |
109 | 03:05 < t0m> 4) merge&profit |
110 | 03:05 < t0m> *conversion |
111 | 03:05 < dhoss> rearrange meaning mv foo MyApp? |
112 | 03:06 < t0m> erm, yes, as described above, so that the contents of share/ is |
113 | laid out the same as a newly generated MyApp |
114 | 03:06 < dhoss> OH |
115 | 03:06 < dhoss> okay |
116 | 03:06 < dhoss> instead of my current fuckery :-) |
117 | 03:06 < dhoss> autarch: btw if you have an idea for that i'd love an email to |
118 | the one in topic |
119 | 03:06 < t0m> cause then the entire generate application becomes 'copy this |
120 | directory from X => Y, applying some file mangling rules) |
6be0f534 |
121 | |
122 | ============================================================================= |
123 | 04:26 < t0m> dhoss: erm, yeah - so the two things to do is: unit tests for the |
124 | methods like get_file which we're not actually using any more.. |
125 | 04:27 < t0m> (do that by making t/lib/ExampleHelper.pm - copy the TT one from |
126 | CPAN or something, and then write some tests that get_file can |
127 | successfully pull chunks out the __DATA__ segment |
128 | 04:28 < t0m> and rename everything in the sharedir to be named as-per where it |
129 | ends up in your app, but suffixed by .tt or .bin |
130 | 04:29 < t0m> e.g. share/Makefile.PL.tt share/root/favicon.ico.bin |
131 | share/lib/MyApp.pm.tt |
132 | 04:30 < t0m> the exact reasons for this nameing scheme become aparent post |
133 | first merge, when we remove most of the code :) |
134 | 04:30 < t0m> but all of share/ needs to be in it's final resting place pre-merge |
135 | 04:31 < marcus> http://lumberjaph.net/blog/index.php/2009/05/30/catalystxdispatcherasgraph/ # <3 lazyweb |
136 | 04:31 < t0m> so tests the stuff we're no longer using in Helper.pm (e.g. |
137 | get_file) still works. Rename share/ bits |
138 | 04:31 < t0m> marcus: Indeed :) |
139 | 04:31 < t0m> ^^ those two, then first merge. |
9b5ece3f |
140 | 23:36 <@kd> dhoss: just found a bug in ::Devel ... |
141 | 23:36 <@kd> we're using #!/usr/bin/env perl now |
142 | 23:36 <@kd> that means we have to add the use warnings; pragma beneath it |
143 | 23:37 <@kd> that doesn't appear to be happeining in the scripts |
144 | 23:37 <@kd> please fix |
145 | 23:38 <@kd> dhoss: i.e. http://gist.github.com/122081 |
5c330d3c |
146 | ============================================================================== |
99d7d5b9 |
147 | |
148 | # DONE |
5c330d3c |
149 | 1. Rename everything into final layout: |
150 | . tt files all named .tt |
151 | . none tt files all named .bin |
152 | . directory structure as generated, e.g. share/lib/MyApp.pm.tt, share/root/favicon.ico.bin |
153 | |
154 | 2. get new layout working |
155 | |
99d7d5b9 |
156 | |
157 | # DONE |
5c330d3c |
158 | 3. get tests for back compat - i.e. the methods no longer used in Helper.pm like 'get_file' |
159 | . copy Catalyst::Helper::View::TT into t/lib/TestBackCompat.pm |
160 | . write test t/deprecated_methods_backcompat.t which says: |
161 | use FindBin qw/$Bin/; |
162 | use lib "$Bin/lib"; |
163 | use Test::More tests => 1; |
164 | |
165 | my $helper = TestBackCompat->new( %maybe_some_params_here ); |
166 | my $a_section = $helper->get_file('FileName'); |
167 | is $a_section, 'FILECONTENTSFROM__DATA__'; |
168 | |
99d7d5b9 |
169 | 3. RFC - DONE |
5c330d3c |
170 | |
171 | Looking ahead |
172 | |
173 | MooseX::Getopt |
6be0f534 |
174 | |
b87c5403 |
175 | ################################################################################ |
176 | 17:46 < t0m> dhoss: erm, oh, so the _other_ thing I didn't think of |
177 | 17:46 < t0m> Is Cat works on windows |
178 | 17:46 < t0m> I have stuffed loads of "t/foo.t" |
179 | 17:46 < t0m> into strings. |
180 | 17:46 < t0m> which will break win |
181 | 17:47 < t0m> so, all those places where I just "string/append/a/file.name" |
182 | 17:47 < t0m> you need to use Path::Class qw/file/; file(qw/ string append a |
183 | file.name /); |
184 | 17:47 < t0m> like ^^^ |
185 | 17:48 < t0m> then it will still work on win32 |
186 | 17:48 < dhoss> noted |
187 | 17:48 < t0m> There is already File::Spec->catfile |
188 | 17:48 < t0m> which does the same thing |
189 | 17:48 < t0m> but more ugly |
190 | 17:48 < t0m> either/or |
191 | 17:48 < purl> either/or is "public load_first_existing_class" or "load_class |
192 | passes multiple args to load_first_existing_class" |
193 | 17:48 < t0m> don't care... |
194 | 17:49 < t0m> Just "file/name" is BAD |
195 | 17:49 < t0m> and will hate windows users |
196 | |
197 | Summary: |
198 | 17:43 < dhoss> 1. fix pod coverage, 2. remove TestAppForInvocation 3. ??? |
199 | 17:43 < t0m> 3. merge |
200 | |
201 | |
c5387711 |
202 | |
203 | |