Skip to content

Conversation

bf4
Copy link
Contributor

@bf4 bf4 commented Jul 25, 2014

Part of merging development from https://github.com/bf4/code_metrics
and #11148
back into Rails

Next steps, if given go ahead, is to create an internal
code_tools gem, either in the tools directory, or inside railties
that

  • is not part of the Rails release cycle
  • is used internally by Rails
  • does not depend on Rails

See https://github.com/bf4/rails/tree/code_tools_next/tools/code_tools for what this could look like

I would also move into it the code and functionality of
rake stats and rake notes.

@@ -0,0 +1,41 @@
#!/usr/bin/env ruby
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have .rb extension?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just being consistent with the format of that directory..

@egilburg
Copy link
Contributor

If tools are going to be extracted and encapsulated, they should have their own test coverage.

@bf4
Copy link
Contributor Author

bf4 commented Jul 25, 2014

@egilburg Just to be clear, you're advocating that I write tests for the currently untested code? Yeah, that's part of next step.

@matthewd
Copy link
Member

Can you elaborate on what's changed since #11148? I guess I missed something, because that seems to conclude with a pretty strong "no thanks".

@bf4
Copy link
Contributor Author

bf4 commented Jul 26, 2014

@matthewd In the discussion there, @dhh wrote "please do make a PR with any improvements you've made to the code".

That is this PR. (Or at least begins here. There's more to do.)

Some differences:

  1. Remove code_statistics so it can be required as a gem #11148 was about how I was given the go ahead to extract out rake stats, and then, a change of mind not to extract rake stats (or, more likely, responding by email and not remembering the original discussion. See https://twitter.com/dhh/status/377532862896930816 as well).
  2. This PR doesn't touch that. This PR only cleans up existing code.
  3. This PR does suggest making an internal gem, which would avoid the issues brought up in Remove code_statistics so it can be required as a gem #11148, that is, that somehow extracting a dev tool from Rails would break production apps.

I wish we had met at some point so I could get an idea of where you're coming from. Removing code from Rails that isn't part of Rails is kind of a no-brainer up to the point not wanting to break anything. Rails is web app framework. There's no reason it should have rotting metrics tools spread throughout it. And I'm pro metrics (I was prompted to consider this work after watching @tenderlove's recent talk at RedDot Ruby. I'm pretty into metrics. FWIW, I maintain metric_fu, help on simplecov, and have commit to a few others.)

@bf4
Copy link
Contributor Author

bf4 commented Jul 31, 2014

bump? @rafaelfranca @fxn thoughts?

@bf4
Copy link
Contributor Author

bf4 commented Aug 18, 2014

💤

rafaelfranca added a commit that referenced this pull request Aug 18, 2014
Update, unify, encapsulate, and fix various code tools in Rails
@rafaelfranca rafaelfranca merged commit eced6f8 into rails:master Aug 18, 2014
@bf4
Copy link
Contributor Author

bf4 commented Aug 18, 2014

@rafaelfranca wow, thanks!

@rafaelfranca
Copy link
Member

You are welcome.

@bf4
Copy link
Contributor Author

bf4 commented Oct 22, 2014

@rafaelfranca What would you think of a PR creating a railtools gem in the tools directory that just moved the tools/line_statistics and tools/profile code in, somewhat similar to how I had spiked out https://github.com/bf4/rails/tree/code_tools_next/tools/code_tools

@seuros
Copy link
Member

seuros commented Oct 22, 2014

@bf4 👍 but it will be better if the gem was named rails-code_tools

@bf4
Copy link
Contributor Author

bf4 commented Oct 22, 2014

@seuros wow, you're a collaborator now! Congrats! 👯 Is that rails-code_tools such that one would require 'rails/code_tools'? You don't think or railtools is a better name to go along side railties (even though the require for railties is rails)...

@seuros
Copy link
Member

seuros commented Oct 22, 2014

Thank you :).
Rails tools is ambiguous IMO. i think it should be something like rails-code_tools rails-metrics rails-code_metric.... These tools are used to test the framework and should not be confused with minitest/rspec addons.
wdyt ?

@rafaelfranca
Copy link
Member

hmm, not sure if it is worth. What we would gain with this change? These tools are internal.

@bf4
Copy link
Contributor Author

bf4 commented Oct 22, 2014

@rafaelfranca Well, my motivation would be to move the code for rake stats and rake notes into this new rails gem, as well, since they are useful and used tools that they aren't really dependent on rails in any way. And while we're making a rails-code_tools gem, the other scripts in tools would make sense to also go in there for others to use. Rails would then continue to have the code in the repo, but it would be installable independently of the other bundled rails gems.

If we think of Rails gems in terms of responsibilities, and their responsibilities being simplified and broken down into smaller gems (e.g. actionpack an actionview), this is a natural progression, as railties is really where the Rails web app is defined, and is not the natural place for tooling.

@rafaelfranca
Copy link
Member

I'm all the copy the code here and create a new gem, but I'm 👎 for creating a new gem inside Rails repository. This will create extra work for doing releases for something that I don't believe it is worth.

@bf4 bf4 deleted the code_tools branch October 6, 2016 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants