gregnavis / active_record_doctor

Identify database issues before they hit production.
MIT License
1.72k stars 56 forks source link

Idea: Allow the configuration to be named `.active_record_doctor.rb` (`.rb` suffix) to assist editors #159

Closed jdufresne closed 9 months ago

jdufresne commented 9 months ago

Many editors use the filename suffix to decide how to highlight syntax and load other language hooks.

The name .active_record_doctor leaves this as ambiguous to an editor, so it may not automatically select Ruby syntax highlighting.

There are relatively easy workarounds to this problem by either configuring one's editor to special case .active_record_doctor or selecting the editor's syntax highlighting manually, but renaming the file would be more universal and friendly to new users.

Other Ruby lint tools, such as rubocop, would also pick up the suffix automatically.

fatkodima commented 9 months ago

I think this is a good idea. I am a little tired to "select the syntax highlighting" in sublime text. If you would like to create a PR, let me know. Otherwise, I can do that.

jdufresne commented 9 months ago

Another thought is to use a YAML file instead: .active_record_doctor.yml. AFAICT, the configuration is entirely declarative and shouldn't normally require the full power and logic of Ruby.

fatkodima commented 9 months ago

It was made to be a ruby file on purpose to allow such logic.

jdufresne commented 9 months ago

It was made to be a ruby file on purpose to allow such logic.

I understand, but it doesn't seem like it would normally provide a practical benefit. Do you have a real life use case where it does?

fatkodima commented 9 months ago

I hadn't had a use case yet where it wouldn't be done in the yml format. But it is already implemented to be parsed as ruby and don't think this will (or makes sense to) change. With ruby format, it is easily, for example, to implement some custom logic: 1) something like rubocop's "todo" - you define some configs in a separate .active_record_doctor_todo file and include it in the main file 2) doing some custom logic to detect which tables to check etc

With yml something will be impossible or there would be a need to wait for some custom syntax to be implemented.

fatkodima commented 9 months ago

You actually can currently use the custom extension of the file via

# Rakefile
require "active_record_doctor"

ActiveRecordDoctor::Rake::Task.new do |task|
  ...
  task.config_path = ::Rails.root.join(".active_record_doctor.rb")
  ...
end
jdufresne commented 9 months ago

I agree that would work, unfortunately it requires more configuration on the user's part. IMO, it would be better if it worked out of the box.

gregnavis commented 9 months ago

@jdufresne, thank you for opening the issue. I think your suggestion makes total sense and will improve user experience.

Regarding YAML, building a Ruby solution a certain advantages: it's much more extensible out of the box. For example, if we decide to support custom ignore rules in the future it would be a matter of passing a proc or a block in the configuration file. With YAML, it would require some kind of custom implementation.