Prerelease annotate-models 2.5.0 pre 1

[Alex Chaffee]

I'd like to (finally!) do a prerelease of annotate_models sometime
today, based off the current ctran master branch. Any objections?

Note that since it's a prerelease, you'll have to specify the version
explicitly, e.g.

in Gemfile:
gem "annotate", "~>2.5.0.pre1"

from command line:
gem install --pre annotate

(or something like that -- ymmv)

If there are more commits we really want to include before going live,
please let's discuss on this thread.

MrJoy, it looks like you've been busy! Is it going to be painful to
merge/rebase your branch into ctran's?

--
Alex Chaffee - al...@stinky.com
http://alexchaffee.com
http://twitter.com/alexch

[Alex Chaffee]

Just pushed 2.5.0.pre2. Getting closer to a real release! Please try it out.

On Friday, June 8, 2012 10:45:40 AM UTC-7, Alex Chaffee wrote:

I'd like to (finally!) do a prerelease of annotate_models sometime
today, based off the current ctran master branch. Any objections?

Note that since it's a prerelease, you'll have to specify the version
explicitly, e.g.

in Gemfile:

gem "annotate", "~>2.5.0.pre2"

[Jon Frisby]

Sorry, for some reason I had notifications turned off for this group or something...

So, my work as of June?  Dunno about difficulty of merge.

My work *since then*?  Well, I've merged all your latest in, but there are a couple controversial points to discuss.

1) I still think defaulting to sorted order is THE way to go, having worked in a multi-person, multi-branch team enough to have been caught in the peculiar hell of bouncing sort orders. Your attempts to make things "sedentary" in their positions feels like a stopgap.

2) I encountered a LOT of issues around old versions of model code being in memory when annotation happens, so I decided to punt and basically make the rake tasks set env vars and call the shell script(s) to ensure they've got the latest code.

3) I ditched the ObjectSpace approach and with it, all the bad juju with FactoryGirl, JRuby, et al.  Instead, I forcibly eager-load classes (using strategies that work for both Rails 2.3 and Rails 3.x), then look for ActiveRecord::Base.subclasses.  There's a lot of code simplification that could come out of this but I haven't gotten there yet.

4) I split the binary into annotate_models and annotate_routes.  It neatly gets around the issue of annotate being ambiguous, while avoiding the issue of annotate_models being misleading.  And I think it's safe to say we can afford to ditch backwards compatibility on that name.

5) I revamped the integration testing system.  RVM is now required, but this comes with a slew of benefits:

  a) It's easier to set up complex test scenarios -- for example, I have one with no asset pipeline, one with FactoryGirl set to autoload, etc.

  b) You can "tinker" more easily instead of running formal tests as instead of simple Gemfile/Gemfile.lock pairs, I'm basically making super-stripped-down Rails apps that get copied en masse into place for test runs.  The .rvmrc, Gemfile, and Gemfile.lock are templated, such that the repo version is good for you to just cd in and play around, and at rspec-time it'll write ones appropriate for that.  The beautiful part is that both should inherit your currently active Ruby and use a private gemset.  (I'm testing that out as I write this...)

  c) I downgraded Rake on the Rails 2.3 test to be Rake 0.8.7.  Reduces a ton of noise, and is more typical of legacy setups I think.

  d) Incremental runs are much faster since they don't need to reinstall all the gems if you want to maintain a clean operating/development environment.

These integration setups are quite small -- ignoring the .gitkeep files -- the Rails 2.3 one is 3 templates plus 28 files.  The Rails 3.2.2 one is 3 templates plus 34 files.  The Rails 3.2 + FactoryGirl one is 28 files + 3 templates.  Etc etc etc.

6) Route annotations can now be at the head or foot of a file, are robust about whitespace, even when changing positions, and can be removed programmatically.

7) I switched from mg and rdoc to Jeweler and yard.  The change to Jeweler is a bigger deal to me:  I've been fighting some strange tools in JRuby-land that like to do things like package up gems separately from their .gemspec file, so trying to refer to gem code from the gemspec just does not work *at all* (and using git on the fly won't work when all the files are in a jar).  I also tweaked the existing history and todo list to be proper rdoc syntax and added them to the docset so running "rake yard" will include them.

8) I've overhauled code-loading quite a bit to be cleaner and more manageable, but at present there's a strong assumption of Bundler being involved.  That needs to change.  I just didn't have time to code the fallback path.

I also added lots of tests, blah blah blah.  Doing coverage analysis will be slightly challenging with the shelling-out aspect, but I think I have an idea in mind for that.

Benchmarks on the suite (which comprises 58 examples, with each of the 5 Rails integration tests being one example -- and incremental runs, after a couple to do bundle installs and prime disk caches, not first-runs):

1.9.3-p194: 19s

1.9.2-p290: 32s

1.9.2-p180: 26s

1.8.7-p370:

rbx-2.0.testing*: N/A

jruby-1.7.0.preview2**: 3m15s (!!!)

* - Not compatible.  "undefined method `<=>' on id:Symbol.", among many other issues.

** - Failed on integration tests, requiring some Gemfile magic, and on the stack-trace test (not bothering to suss that out just now).  Plus, I had to comment out pry/pry-coolline from the main project Gemfile to use these for a full rspec run, despite compartmentalizing them to platform => ruby.  Perhaps I need to be more specific about that...  Feh.

On Friday, June 8, 2012 10:45:40 AM UTC-7, Alex Chaffee wrote:

[Alex Chaffee]

> 1) I still think defaulting to sorted order is THE way to go, having worked
> in a multi-person, multi-branch team enough to have been caught in the
> peculiar hell of bouncing sort orders. Your attempts to make things
> "sedentary" in their positions feels like a stopgap.

The counter argument is that order is meaningful; people expect to see
more recently added columns near the bottom of the list. And can't you
escape that hell by making everyone run rake db:reset?

Also, schema.rb is generated using the same db-order ordering. Is that
causing the same problems?

> 2) I encountered a LOT of issues around old versions of model code being in
> memory when annotation happens, so I decided to punt and basically make the
> rake tasks set env vars and call the shell script(s) to ensure they've got
> the latest code.

I'm all for improving isolation, and I think the current code is in a
peculiar middle state between "load the whole rails environment" and
"don't load rails, just try to parse model files one at a time". Also
I'd love to get the rake task to be a pure client of the annotate
script (now there's a lot of rake leakage into the core annotation
code).

I don't know what you mean by "old versions of model code", though.

> 3) I ditched the ObjectSpace approach and with it, all the bad juju with
> FactoryGirl, JRuby, et al. Instead, I forcibly eager-load classes (using
> strategies that work for both Rails 2.3 and Rails 3.x), then look for
> ActiveRecord::Base.subclasses. There's a lot of code simplification that
> could come out of this but I haven't gotten there yet.

I think the bad juju has all been exorcised but I like the sound of this.

> 4) I split the binary into annotate_models and annotate_routes. It neatly
> gets around the issue of annotate being ambiguous, while avoiding the issue
> of annotate_models being misleading. And I think it's safe to say we can
> afford to ditch backwards compatibility on that name.

I disagree. "annotate" is so short and punchy and clear, and there's
now a fair bit of history around it too. The only problem is that
imagemagick has a tool with the same name but I think "bundle exec
annotate" can work around that.

I do agree that annotating routes was feature creep but I don't think
it's hurting much, relatively speaking.

> 5) I revamped the integration testing system. RVM is now required, but this
> comes with a slew of benefits:
> a) It's easier to set up complex test scenarios -- for example, I have one
> with no asset pipeline, one with FactoryGirl set to autoload, etc.
> b) You can "tinker" more easily instead of running formal tests as instead
> of simple Gemfile/Gemfile.lock pairs, I'm basically making
> super-stripped-down Rails apps that get copied en masse into place for test
> runs. The .rvmrc, Gemfile, and Gemfile.lock are templated, such that the
> repo version is good for you to just cd in and play around, and at
> rspec-time it'll write ones appropriate for that. The beautiful part is
> that both should inherit your currently active Ruby and use a private
> gemset. (I'm testing that out as I write this...)
> c) I downgraded Rake on the Rails 2.3 test to be Rake 0.8.7. Reduces a
> ton of noise, and is more typical of legacy setups I think.
> d) Incremental runs are much faster since they don't need to reinstall all
> the gems if you want to maintain a clean operating/development environment.
>
> These integration setups are quite small -- ignoring the .gitkeep files --
> the Rails 2.3 one is 3 templates plus 28 files. The Rails 3.2.2 one is 3
> templates plus 34 files. The Rails 3.2 + FactoryGirl one is 28 files + 3
> templates. Etc etc etc.

That all sounds very nice. How hard would it be to roll out a new
template for a new release of Rails?

I'm not sure about downgrading rake. I was thinking of monkey-patching
rake and rubygems to get rid of some of the Rails 2 noise.

> 6) Route annotations can now be at the head or foot of a file, are robust
> about whitespace, even when changing positions, and can be removed
> programmatically.

Also nice.

> 7) I switched from mg and rdoc to Jeweler and yard. The change to Jeweler
> is a bigger deal to me: I've been fighting some strange tools in JRuby-land
> that like to do things like package up gems separately from their .gemspec
> file, so trying to refer to gem code from the gemspec just does not work *at
> all* (and using git on the fly won't work when all the files are in a jar).
> I also tweaked the existing history and todo list to be proper rdoc syntax
> and added them to the docset so running "rake yard" will include them.

Yard is fine but I've had the opposite experience with Jeweler. A lot
of what Jeweler did is now part of Rubygems (gem) and mg just calls
that. I find Jeweler just adds unnecessary complexity these days.

What do you mean by trying to refer to gem code from the gemspec? The
only code I ever use from inside a non-generated gemspec is
Foo::Version (or Foo::VERSION) and you can cleanly require just that
file if you isolate it.

> 8) I've overhauled code-loading quite a bit to be cleaner and more
> manageable, but at present there's a strong assumption of Bundler being
> involved. That needs to change. I just didn't have time to code the
> fallback path.

Since Rails requires Bundler now, I wouldn't feel too bad about that
assumption, but if it's possible to do without it, that would be best.

I wonder if you can put some of the non-controversial changes into
separate branches and make pull requests for them, then we can keep
bickering about the rest :-)

[Jon Frisby]

On Friday, August 17, 2012 11:26:49 AM UTC-7, Alex Chaffee wrote:

> 1) I still think defaulting to sorted order is THE way to go, having worked
> in a multi-person, multi-branch team enough to have been caught in the
> peculiar hell of bouncing sort orders. Your attempts to make things
> "sedentary" in their positions feels like a stopgap.

The counter argument is that order is meaningful; people expect to see
more recently added columns near the bottom of the list. And can't you
escape that hell by making everyone run rake db:reset?

I don't think I've ever thought in terms of expecting to see more recent columns near the bottom of the list.  It's taken me years to ignore the aggravation of Rails not allowing me to control the ordering explicitly.  If anything, alphabetical sorting means I can visually do a binary search for the column name I'm looking for, so I've kinda gotten used to that idea.

And no, I really can't force it.  Multiple, distributed teams, working across a variety of time zones and branches, with different schema changes happening on different branches.  And that's with just 7 or 8 committers right now.  The only solution would be having people periodically drop and recreate their local DB, which is pretty onerous.

If Rails exposed clear metadata about column order as it exists in the (explicitly ordinal) migrations, then we could produce a stable result without sorting, but short of reproducing a canonical ordering by running the migrations against a scratch, empty database, having sorting off by default just leads to people suddenly experiencing wacky little results which they may initially dismiss as odd but not significant enough to worry about (columns flip-flopping order from branch to branch, with interesting consequences for merges), until finally someone gets fed up and says WTF?! and starts digging into the root cause.

I.E. it's a problem you grow into subtly, and the solution may not be totally obvious -- so I prefer the solution that very easily guarantees it never becomes an issue.

Also, schema.rb is generated using the same db-order ordering. Is that
causing the same problems?

I suspect Rails does some order-stabilizing (not *moving* columns that are already in schema.rb), rather than using one's local DB order exclusively, but on occasion, yes it has.  In one case, even dropping local DBs wasn't enough because production had been migrated out-of-canonical-order, and so people would pull down a production snapshot to test against real data with, and have a wonky ordering that would show up the next time they made schema changes.

The resolution to that issue was a MySQL-specific migration that explicitly re-ordered a few columns in terms of others.  It was, frankly, a royal PITA.

 

> 2) I encountered a LOT of issues around old versions of model code being in
> memory when annotation happens, so I decided to punt and basically make the
> rake tasks set env vars and call the shell script(s) to ensure they've got
> the latest code.

I'm all for improving isolation, and I think the current code is in a
peculiar middle state between "load the whole rails environment" and
"don't load rails, just try to parse model files one at a time". Also
I'd love to get the rake task to be a pure client of the annotate
script (now there's a lot of rake leakage into the core annotation
code).

My changes definitely help move in that direction, although at the cost of "load the whole rails environment".  However I now have all sorts of corner-cases working properly, including models-from-engines being properly ignored, models defined in multiple places being annotated in all places, no magic monkey-patching causing issues (see the bug report about auto-loaded FactoryGirl interfering with the ObjectSpace approach), STI, namespacing, etc.

About the only edge-case I'm aware of that I have not explicitly tested yet is custom inflections, and I'll probably have that covered this weekend.

I don't know what you mean by "old versions of model code", though.

Sorry, versions initialized against old versions of the schema.  So for whatever reason, model X gets loaded, then migration Y runs (changing the schema underlying X), then annotate runs -- model X won't have the new attributes/indexes/whatever loaded into ActiveRecord's metadata during that run of Rake.  So a common problem we were experiencing with annotate was that the annotations would "lag" a migration or two behind reality unless you explicitly ran db:migrate twice.

 

> 3) I ditched the ObjectSpace approach and with it, all the bad juju with
> FactoryGirl, JRuby, et al.  Instead, I forcibly eager-load classes (using
> strategies that work for both Rails 2.3 and Rails 3.x), then look for
> ActiveRecord::Base.subclasses.  There's a lot of code simplification that
> could come out of this but I haven't gotten there yet.

I think the bad juju has all been exorcised but I like the sound of this.

> 4) I split the binary into annotate_models and annotate_routes.  It neatly
> gets around the issue of annotate being ambiguous, while avoiding the issue
> of annotate_models being misleading.  And I think it's safe to say we can
> afford to ditch backwards compatibility on that name.

I disagree. "annotate" is so short and punchy and clear, and there's
now a fair bit of history around it too. The only problem is that
imagemagick has a tool with the same name but I think "bundle exec
annotate" can work around that.

Ugh.  I loathe having to do that.  As do many developers -- hell, even RVM is shipping with rubygems-bundler pre-installed into the global gemset for all rubies (which I promptly kill, to avoid nasty compatibility issues -- but people are willing to bear some such issues out to avoid the bundle exec solution; my solution is to actually use gemsets properly, keep them tidy, and not have it ever become an issue at all).

Also, the bundle exec option falls apart when you want to automate anything.  One thing bundler does is override system() and `` behavior, such that spawned sub-shells have an environment that doesn't require explicit use of "bundle exec", so if you want to automate RMagick from within your Rails app (say, a Rake task to some common transformative steps on static assets), it's now ambiguous as to which tool will be executed when you start your sub-shell.

 

I do agree that annotating routes was feature creep but I don't think
it's hurting much, relatively speaking.

Eh?  Didn't mean to imply it's feature-creep.  I think it's a natural fit; I just think the name "annotate_models" is confusing in that context.

I actually think there's more annotations worth adding, like summaries of attribute scoping metadata.  I.E. a section showing what attributes are mass-assignable, what are protected, what are read-only, and what named mass-assignment scopes exist.  However, that's considerably more work, more scope, so I didn't just go ahead and build it.

 

> 5) I revamped the integration testing system.  RVM is now required, but this
> comes with a slew of benefits:
>   a) It's easier to set up complex test scenarios -- for example, I have one
> with no asset pipeline, one with FactoryGirl set to autoload, etc.
>   b) You can "tinker" more easily instead of running formal tests as instead
> of simple Gemfile/Gemfile.lock pairs, I'm basically making
> super-stripped-down Rails apps that get copied en masse into place for test
> runs.  The .rvmrc, Gemfile, and Gemfile.lock are templated, such that the
> repo version is good for you to just cd in and play around, and at
> rspec-time it'll write ones appropriate for that.  The beautiful part is
> that both should inherit your currently active Ruby and use a private
> gemset.  (I'm testing that out as I write this...)
>   c) I downgraded Rake on the Rails 2.3 test to be Rake 0.8.7.  Reduces a
> ton of noise, and is more typical of legacy setups I think.
>   d) Incremental runs are much faster since they don't need to reinstall all
> the gems if you want to maintain a clean operating/development environment.
>
> These integration setups are quite small -- ignoring the .gitkeep files --
> the Rails 2.3 one is 3 templates plus 28 files.  The Rails 3.2.2 one is 3
> templates plus 34 files.  The Rails 3.2 + FactoryGirl one is 28 files + 3
> templates.  Etc etc etc.

That all sounds very nice. How hard would it be to roll out a new
template for a new release of Rails?

I haven't automated it yet, but my general approach here has been to copy-pasta one of the existing integrations, and make the relevant mass substitutions, nuke the Gemfile.lock and rebuild it, then tweak that to be a template.  Depending on the complexity of the scenario involved, it's a few minutes of work once you know what needs to be changed and where.

I.E. it definitely bears a little automation on the generation-side of things, but it feels very achievable to automate the process of kickstarting a barebones integration test to the point that you can customize it to the desired behavior trivially.

 

I'm not sure about downgrading rake. I was thinking of monkey-patching
rake and rubygems to get rid of some of the Rails 2 noise.

It's only downgraded in the 2.3 test.  And we could easily have a 2.3 integration test that uses a newer build of Rake as well.  Point is though that it felt like a good fit WRT regression-testing, in terms of the likely 'real world' usage scenarios for 2.3 users.

 

> 7) I switched from mg and rdoc to Jeweler and yard.  The change to Jeweler
> is a bigger deal to me:  I've been fighting some strange tools in JRuby-land
> that like to do things like package up gems separately from their .gemspec
> file, so trying to refer to gem code from the gemspec just does not work *at
> all* (and using git on the fly won't work when all the files are in a jar).
> I also tweaked the existing history and todo list to be proper rdoc syntax
> and added them to the docset so running "rake yard" will include them.

Yard is fine but I've had the opposite experience with Jeweler. A lot
of what Jeweler did is now part of Rubygems (gem) and mg just calls
that. I find Jeweler just adds unnecessary complexity these days.

Take a peek at how I made use of it.  I have a pattern for using it that gives me a nice degree of control, and results in a clean, standalone .gemspec that doesn't have any dependencies on git, or the relative position of the .gemspec to the code.  It also allows me to keep the Gemfile as the central authority for what the project depends on in what contexts, without doing stupid things like making the project's test-mode dependencies be a part of the test-mode dependencies of the consumer of the project.

 

 

What do you mean by trying to refer to gem code from the gemspec? The
only code I ever use from inside a non-generated gemspec is
Foo::Version (or Foo::VERSION) and you can cleanly require just that
file if you isolate it.

The use of Dir.glob / `git ls-files`, and requiring any files cause the interpretation of the gemspec to be only viable if it is located in the root directory of where the gem was expanded to.  The Dir.glob / `git ls-files` part also only works when the gem is expanded onto the FS (which isn't the case when your app has been compiled into one mega-jar file in jruby) -- and the `git ls-files` part even requires that git be present on machines where the gem is *deployed*.  That's a reasonable assumption for a development environment but not necessarily for a production environment.

I'm not opposed to using something *else* besides Jeweler -- my only objective is to make it so the .gemspec is a generated artifact and not a piece of meta-code.  It's just too much of a PITA as soon as you're out of MRI+Bundler land.

> 8) I've overhauled code-loading quite a bit to be cleaner and more
> manageable, but at present there's a strong assumption of Bundler being
> involved.  That needs to change.  I just didn't have time to code the
> fallback path.

Since Rails requires Bundler now, I wouldn't feel too bad about that
assumption, but if it's possible to do without it, that would be best.

I'm mostly thinking about it in terms of Rails 2.x compatibility.  I've been able to preserve 2.3 compatibility with my changes quite readily, with only a few specific little quirks...

 

I wonder if you can put some of the non-controversial changes into
separate branches and make pull requests for them, then we can keep
bickering about the rest :-)

That... would be a bit messy at this point.  I mean, I can flip around the sorting default easily enough, and the yard/jeweler switch could also be surgically avoided, but things like the split to two binaries would require a fair bit more work, as would backing out the loading of the whole Rails env.

The output of:

  git diff -M -C ef70622..78a5a3f --shortstat

Is:

   200 files changed, 5064 insertions(+), 967 deletions(-)

... which is pretty significant, given that my branch has 205 files under version control, totaling 6,431 lines.  Granted most of that is the five separate integration tests -- 178 files, 4,260 lines, but still -- it's a significant changeset and many of the changes necessarily touch the same parts of the code.

Also, since my last mail:  I managed to refactor things a bit, and remove a fair chunk of now-dead code (another side-effect of load-the-rails-env-and-proceed-from-there) as well as adding support for multiple model directories, made --require work properly with annotate_routes and a few other fixes.

There's a couple broken specs related to your --trace option and the behavior when say, a model can't be annotated; I just need to look into what is meaningfully correct behavior here.  In a couple cases I intentionally don't bother producing output (I.E. when a model file can't be found in the model-dir search path, it must be coming from an engine/plugin/gem of some sort), etc.

-JF

[Jon Frisby]

Ok, it occurred to me that we don't need ALL the scaffolding for the integration tests, so I just nuked app/controllers, app/views, and test/functional from all the integration tests in my branch, which eliminates a lot of cruft.

The integration tests now account for 132 files, totaling 2,931 lines, and the results of:

  git diff -M -C ef70622..538eee8 --shortstat

Are:

   154 files changed, 3651 insertions(+), 998 deletions(-)

So, slightly more representative of the real scope of the changes.

-JF

[Jon Frisby]

Aaand, I just pushed a branch to my repo -- incremental_integration.  It brings over a bunch of the smaller, less controversial changes:

1) Move to YARD from RDoc.

2) Some minor formatting / dead code / whitespace tidy-ups.

3) Nicer RSpec defaults.

4) DRY up fileset handling so we don't have inconsistencies about what's handled in which tasks.

5) Implement the namespaced models test.

6) Add explicit CLI options for each position parameter.

7) Add some helpers for testing with Rubinius.

8) Fix a bug where the wrong Rakefile could be loaded.

9) All my fixes/improvements to routes.rb annotation, which includes a fix for handling newer versions of Rake (whose first line of output doesn't need to be chopped off).

10) Make --require work for all four task types.

11) Guard tasks from being loaded multiple times.

12) Some improvements to consistency/DRYness of option handling.

$ git diff -M -C f094da6..8927c88 --shortstat

 23 files changed, 617 insertions(+), 273 deletions(-)

I'll see about bringing over my code-loading changes too, after I finish some work-related stuff.

-JF

[Alex Chaffee]

Nice! I'll take a look, um, you know, soon :-)

One question occurred to me: does it still work in non-Rails
ActiveRecord projects?

> --
> You received this message because you are subscribed to the Google Groups
> "annotate-models" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/annotate-models/-/rLS0HlWyZvkJ.
>
> To post to this group, send email to annotat...@googlegroups.com.
> To unsubscribe from this group, send email to
> annotate-mode...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/annotate-models?hl=en.

[Jon Frisby]

My main branch does not, but then that appears to have been pretty horribly broken for some time. I don't think the incremental_integration branch is any worse off in that regard, although I haven't tried it...

Restoring that functionality is on my short list due to some internal efforts at my day job. It'll probably involve an eager-loading process that just scans for all .rb files in the model-dir(s) the user has specified, blindly require'ing them, then looking for subclasses of ActiveRecord::Base as I do now. I.E. it will continue to assume the same filename<-->classname convention that Rails has, but will shy away from other Rails-isms when Rails isn't present

I would appreciate if these changes could be landed sooner rather than later, perhaps as a 2.5.1 release... I want to reconcile and land the remainder of my desired improvements in an acceptable way and that'll be easiest after this stuff gets sorted out. On my master branch I've switched from --no-sort to --sort, and am looking into a better way of generating the gemspec than Jeweler, to minimize complexity while avoiding having the gemspec being 'smart'...

I'll also add an annotate binary back in as a thin wrapper around annotate_models / annotate_routes, so that those who prefer the shorter name can use it if they don't have the ImageMagick installed.

-JF

[Jon Frisby]

 So, in switching my Rails project to Yard, I noticed that the Markdown we're generating (MultiMarkdown, actually) is pretty broken in a number of ways:

1) Doing "**foo     **" doesn't work.

2) NOT wrapping table_or_column_names_with_underscores in `backticks` plays havok with formatting (as _ is used for italics)

3) Empty backticks (``) appear as literal values.

So I took the opportunity to make it work end-to-end in a reasonable way, and added docs explaining the use of MultiMarkdown and suggesting a specific Markdown processor that will work with the results (kramdown), including how to configure yard to play nice with output from annotate.

I also got around to adding an "annotate" binary back in, and changing the docs accordingly.  So now, there are three binaries, and people can use whichever one(s) they want.  The rake tasks will continue to use annotate_models to avoid painful collisions.

So the questions for you:

1) The changes I've made so far on the incremental_integration branch -- are you happy with those, and prepared to merge them?

2) How do you feel about the code loading changes on my master branch (a fairly radical change -- loading the whole Rails env, but ostensibly fixing pretty much all annoying issues surrounding model-vs-file names, namespacing, model configuration, pluralization behaviors, etc.

3) How do you feel about my changes to integration testing on my master branch? (you might want to answer that AFTER my next two integration suites land -- one for inflections, one for non-Rails projects)

4) How do you feel about the Markdown formatting changes on my master branch?  (There's some changes beyond merely fixing the behavior...)

I still need to address your concerns about gemspec generation of course...  Anything else that's outstanding that you consider an impediment to reintegration?

-JF

>> To post to this group, send email to annotate-models@googlegroups.com.

>> To unsubscribe from this group, send email to

>> annotate-models+unsubscribe@googlegroups.com.

>> For more options, visit this group at
>> http://groups.google.com/group/annotate-models?hl=en.
>
>
>
> --
> Alex Chaffee - al...@stinky.com
> http://alexchaffee.com
> http://twitter.com/alexch
>
> --
> You received this message because you are subscribed to the Google Groups "annotate-models" group.

> To post to this group, send email to annotate-models@googlegroups.com.
> To unsubscribe from this group, send email to annotate-models+unsubscribe@googlegroups.com.

[Jon Frisby]

Aaaand, standalone behavior is supported (you don't even need a Rakefile), albeit without the handy generator for setting up a config, which is annoying.

I'm thinking of switching to a dotfile for configuring annotate, instead of a rake task.  Thoughts?

Also, I tightened the surface area of the existing integration tests up a LOT by clipping out Rails subsystems we don't care about in tests (so instead of having *a* test that tests it without the asset pipeline, now there's *a* test that tests it *with* the asset pipeline).  And, I changed a lot of common files to symlinks into a fixtures directory -- I've made a task to help with that process too.

I also added that integration test for standalone behavior...

I've gone ahead and landed a bunch of more aggressive changes to incremental_integration (the new test system, changes to options handling, and some lighter code loading changes to restore non-Rails usage), but if you're not happy with them yet you could merge a couple commits back...

[Jon Frisby]

Ok, on my development branch (decided to move off of master):

1) Switched back to mg from Jeweler, but added a gem:gemspec task that synthesizes the gemspec.  We can make that a dependency for the gem, gem task to ensure things stay in sync at release-time.

2) Bumped the version # to 2.6.0b.beta1, and moved the relevant swath of the changelog up to a new section for that.

3) Correct dependencies -- we depend on ActiveRecord, and Rake -- not merely Rake, or Rake + ActiveSupport.

[Alex Chaffee]

On Sun, Aug 26, 2012 at 7:32 PM, Jon Frisby <jfr...@mrjoy.com> wrote:
> 1) The changes I've made so far on the incremental_integration branch -- are
> you happy with those, and prepared to merge them?

Yes! I'm running tests now and will push to both my and ctran's repo shortly.

I haven't looked at your master branch yet.

[Alex Chaffee]

incremental_integration merged and pushed. Now for some random replies...

> I noticed that the Markdown we're generating (MultiMarkdown, actually) is pretty broken

Sounds likely. It was just a patch I accepted; I'm not really sure why
it's an option. Is it a Yard thing?

>> Also, schema.rb is generated using the same db-order ordering. Is that
>> causing the same problems?
>
> I suspect Rails does some order-stabilizing (not *moving* columns that are
> already in schema.rb), rather than using one's local DB order exclusively,
> but on occasion, yes it has.

So we could try to do the same order-stabilizing...

Or... what about this compromise? Sort alphabetically, except always
put "id" at the top and "created_at" and "updated_at" together at the
bottom?

Also, I notice this feature: "(i.e., don't regenerate if columns
happen to be in a different order. That's just how life is sometimes)"
so your sorting concern may be obsolete.

This whole argument is really begging for a .annotate config file. Or
.annotaterc or annotate_opts.yml or whatever.

>> I don't know what you mean by "old versions of model code", though.
>
> Sorry, versions initialized against old versions of the schema. So for whatever reason,
> model X gets loaded, then migration Y runs (changing the schema underlying X), then
> annotate runs -- model X won't have the new attributes/indexes/whatever loaded into
> ActiveRecord's metadata during that run of Rake.

Yeah, this is the main problem with running inside a Rails
environment. When I first revamped Dave's code (back in like 2006) I
had to be careful *not* to load the Rails environment... but that
meant I had to put in some code to catch macros (e.g. "will_paginate")
that weren't defined. I think I just used method_missing and ignored
it.

This also reminds me of an old trick. If you have a migration that
refers to the model -- let's say to do an AR query to update a field
in the middle of the migration -- but then later on you *rename* that
model, the old migration is still referring to the old name. So you
have to declare a new class *inside* the migration with the same name
as the (old) table. The migration will pick that up and AR will
initialize it based on the current (old) schema.

class ReverseFoos < ActiveRecord::Migration
class Foo < ActiveRecord::Base; end
def up
Foo.all.each{|foo| foo.name.reverse!; foo.save}
end
end

(That works even if the next migration renames Foo to Bar, so there is
no Foo in your app any more.)

Not sure if that trick sheds light on your issue, but it might be a workaround.

> and the `git ls-files` part even requires that git be present on machines where the gem is *deployed*.

That can't be true! I mean, it can, but I think the gemspec is used to
create a .gem file that contains all files that were found when the
.gem file was made.

[Jon Frisby]

Sent from my iPhone

On Aug 27, 2012, at 11:47 AM, Alex Chaffee <al...@stinky.com> wrote:

> incremental_integration merged and pushed. Now for some random replies...
>
>> I noticed that the Markdown we're generating (MultiMarkdown, actually) is pretty broken
>
> Sounds likely. It was just a patch I accepted; I'm not really sure why
> it's an option. Is it a Yard thing?

Not a yard thing specifically, but Yard supports MD as well as RDoc, and I significantly prefer MD (especially MultiMarkdown -- tables are handy!).

>>> Also, schema.rb is generated using the same db-order ordering. Is that
>>> causing the same problems?
>>
>> I suspect Rails does some order-stabilizing (not *moving* columns that are
>> already in schema.rb), rather than using one's local DB order exclusively,
>> but on occasion, yes it has.
>
> So we could try to do the same order-stabilizing...
>
> Or... what about this compromise? Sort alphabetically, except always
> put "id" at the top and "created_at" and "updated_at" together at the
> bottom?

I would be fine with putting PK at the top and timestamps at the bottom. Anything Rails did to stabilize ordering definitely wasn't perfect... I might wind up submitting a patch to AR as well...

>
> Also, I notice this feature: "(i.e., don't regenerate if columns
> happen to be in a different order. That's just how life is sometimes)"
> so your sorting concern may be obsolete.

I'm still trying to think of a suitable test case for this...

>
> This whole argument is really begging for a .annotate config file. Or
> .annotaterc or annotate_opts.yml or whatever.

Yes, yes it is. A rake task is a poor substitute.

Won't help because the class is still initialized with old column defs...

However my master branch solves this by having the rake tasks shell out to the CLI tools, which then aggressively load the whole environment.

This has the further benefit of picking up on things like inflection customizations and the like...

So it's sort of a complete inversion of control -- the CLI tools no longer wrap rake, it's the other way around. Seems quite robust thus far.

>
>> and the `git ls-files` part even requires that git be present on machines where the gem is *deployed*.
>
> That can't be true! I mean, it can, but I think the gemspec is used to
> create a .gem file that contains all files that were found when the
> .gem file was made.

We encountered this in Redstorm, to be precise. The gemspec is packaged as-is with the gem, and some things will try to process it. This works fine if the gemspec is where it ought to be, but if it was moved around (NFI if it's Bundler or Redstorm doing that...) without the right load path set, then things go asplodey in a hurry.

We saw this with the Ripple gem -- we're not packaging Annotate in a Storm topology... But since it seems to be the case that greater than zero use-cases exist where a 'smart' gemspec is bad-juju -- and the API docs for Gem::Specification certainly suggest as much -- then we may as well be tidy if only so someone using annotate as a template for their own gem won't be adopting an iffy practice unwittingly...

>
> --
> You received this message because you are subscribed to the Google Groups "annotate-models" group.

> To post to this group, send email to annotat...@googlegroups.com.
> To unsubscribe from this group, send email to annotate-mode...@googlegroups.com.

[Jon Frisby]

Ok, I found a couple issues...

I've integrated your fixups to date, although I'm a little confused about why the bundler related ones are needed given the .rvmrc I have for integration testing...  Whatever.

1) If you're using rubygems-bundler, it can be the case that binstubs aren't up to date and bundler won't execute.  I detect this situation and correct for it highly selectively in the .rvmrc now.

2) The generated .rake file didn't do the requisite 'require', so rake tasks weren't being loaded properly.  Not actually sure how this was ever working for me...

3) A cruft .gitignore from one of the integration test template apps was laying around.  I removed it.

I've cherry-picked these onto my usual incremental_integration branch.

I also need to track down a possible bug in my code where sometimes changes aren't being applied, plus apply your idea of putting PK fields at the top and timestamps at the bottom when sorting fields, and so forth.  I'll try to get to some of that tonight.

[Alex Chaffee]

RVM is great, but not too useful outside a Unix workstation. So that keeps Windows developers, not to mention Travis CI, out of the loop.

[Jonathon Frisby]

Sure, but switching away from RVM entirely is a bigger step -- this just makes sure we work properly with a more broad array of possible RVM setups.

I'm not opposed to say a Vagrant-based approach here if we can make it fairly light-weight of course, but I'm not sure if that works out any better in the context of things like Travis CI, or Windows…

Thoughts?

-JF

On Dec 3, 2012, at 9:59 AM, Alex Chaffee <al...@stinky.com> wrote:

> RVM is great, but not too useful outside a Unix workstation. So that keeps Windows developers, not to mention Travis CI, out of the loop.
>
>
>