Conversation
flavorjones
left a comment
There was a problem hiding this comment.
@hatsu38 Thank you for submitting this!
If the goal here is to make sure active hash integrates with the CSV gem, can you please add a new spec that verifies this is working as expected? I'd suggest creating a new directory, spec/integration, and a new spec file.
I would also be interested in a short "how to generate CSV" in the README.md file.
|
I would also like to see a short explicit test in |
|
Rails does not have I'd feel more comfortable with just doing the work for column_names. def column_names
field_names.map(&:name)
endAlso looks like |
Using Symbol#name instead of Symbol#to_s avoids creating a new string for each invocation, improving performance and memory efficiency.
Ruby versions prior to 3.0 do not have the name method in the Symbol class.
lib/active_hash/base.rb
Outdated
| field_names.map do |field| | ||
| field.respond_to?(:name) ? field.name : field.to_s.freeze | ||
| end |
There was a problem hiding this comment.
Now field names are all symbols, so I think we can simplify this:
| field_names.map do |field| | |
| field.respond_to?(:name) ? field.name : field.to_s.freeze | |
| end | |
| fields.map(&:name) |
Does that work for you?
There was a problem hiding this comment.
After merging the latest master, the code worked perfectly! 0306858
|
@flavorjones Mike, I almost think dropping Ruby <3.0 (and therefore rails rails <=6.0. It sounds drastic but they are all EOL since early 2023, and this will work for them, just not the new OTOH/ we could use |
|
Oof, hmm. Well for the record: Rails 6.1 is still supported, and it still technically supports Ruby 2.7. Totally annoying. But: you're right that this is a new feature, and Rails 6.1 is only supported for security updates at this point. So I agree it is feasible and probably preferable to drop support for Ruby < 3.0 in the next minor release. |
|
w |
| it "returns an array of column names" do | ||
| expect(Country.column_names).to eq(["name", "iso_name", "size"]) | ||
| end |
There was a problem hiding this comment.
@flavorjones do we want to just conditionally do this?
| it "returns an array of column names" do | |
| expect(Country.column_names).to eq(["name", "iso_name", "size"]) | |
| end | |
| it "returns an array of column names" do | |
| skip unless RUBY_VERSION >= "3.0" | |
| expect(Country.column_names).to eq(["name", "iso_name", "size"]) | |
| end |
There was a problem hiding this comment.
Hard fought. This looks good.
@flavorjones The githubs humbly requests you take a look at this too.
- Add i18n support active-hash#230 @ryu-sato - Add `column_names` method active-hash#311 @hatsu38 - Support Active Record 7.2 active-hash#317 @ashleyHutton - Support ruby 3.4 active-hash#328 @flavorjones - Add `:alias` to `has_many :through` active-hash#329 @alexgriff - Add Active Record 8.0 active-hash#324 @flavorjones - Fix Do not suppress load errors#309 @andreynering - Ensure `field_names` are all strings active-hash#312 @flavorjones - Hide private `add_default_value` active-hash#314 @kbrock - Fix `exists?(nil)` active-hash#320 @y-yagi - Enance Enum support active-hash#321 @hatsu38 - Updated docs active-hash#326 @y-yagi - Drop Active Record < 6.1. Ruby < 3.0 active-hash#324 @flavorjones
Added ===== - Add i18n support active-hash#230 @ryu-sato - Add `column_names` method active-hash#311 @hatsu38 - Support Active Record 7.2 active-hash#317 @ashleyHutton - Support ruby 3.4 active-hash#328 @flavorjones - Add `:alias` to `has_many :through` active-hash#329 @alexgriff - Add Active Record 8.0 active-hash#324 @flavorjones Fixed ===== - Fix Do not suppress load errors#309 @andreynering - Ensure `field_names` are all strings active-hash#312 @flavorjones - Hide private `add_default_value` active-hash#314 @kbrock - Fix `exists?(nil)` active-hash#320 @y-yagi - Enance Enum support active-hash#321 @hatsu38 - Updated docs active-hash#326 @y-yagi Removed ======= - Drop Active Record < 6.1. Ruby < 3.0 active-hash#324 @flavorjones
This pull request adds the column_names method to the ActiveHash gem.
The column_names method is analogous to the one found in Rails' ActiveRecord, allowing users to retrieve the names of columns defined in an ActiveHash model.
Added column_names method to ActiveHash::Base class.
The method returns an array of string column names, similar to ActiveRecord's column_names method.
close #310