Merge fork - blueprinter-ruby/blueprinter to procore-oss/blueprinter #327
Conversation
* Run tests in Github Actions * Update github action configuration Co-authored-by: Ritikesh <[email protected]> * Update ruby versions we run tests against, temporarily run tests on this branch too * do not use ruby 2.6 in tests * Update required ruby version * Try not locking activerecord to as recent a version and run tests in ruby 2.6.9 and 3.2 * Remove this branch from those that trigger tests * Update blueprinter.gemspec --------- Co-authored-by: Ritikesh <[email protected]>
* Allow configuration of array-like classes * update README to include information about the array_like_classes configuration option * do not persist vehicles to db in tests * Consolidate array-like logic and add test case for non-configured array-like class * memoize the array_like_classes * Increment version and update CHANGELOG * Apply suggestions from code review --------- Co-authored-by: Ritikesh <[email protected]>
* create a rubocop workflow * Update rubocop.yml * rubocop V1 * update workflow files * Update rubocop.yml * Update rubocop.yml * Update rubocop.yml * final updates * Update rubocop.yml * final updates * lock rubocop version * update description of gem * Update lib/blueprinter/extractors/block_extractor.rb
update yard and cleanup rubocop.yml workflow
* fix rubocop * run only rspec from tests workflow
…s` (#16) * Removed deprecations on Blueprinter::Field callables * Removed conditional procs with two arguments and modified version and changelog
|
Gotta sign your commits now. Thank you so much for bringing this back into this repo. Looking forward to more contributions. |
|
@jmeridth there are commits from multiple contributors since it's a forward merge as-is. If those aren't signed, not sure how we'd go about that. I don't want to squash them into a single commit and lose meaningful independent commits. What should we do about this then? Can there not be a one-time exception? |
|
We can ignore DCO for that type of scenario. Thank you for clarifying. |
| s.add_development_dependency 'rspec-rails', '~> 6.0' | ||
| s.add_development_dependency 'sqlite3', '~> 1.4.2' | ||
| s.add_development_dependency 'yajl-ruby', '~> 1.4.1' | ||
| s.add_development_dependency 'yard', '~> 0.9.11' |
There was a problem hiding this comment.
I see that we moved the location of gem dependencies from here to the Gemfile as part of this PR. Was that necessary to get everything working? Just trying to understand the intention of the change here!
There was a problem hiding this comment.
development dependencies are better suited for Gemfiles right? it also helps when you want to use different versions of dev dependencies for different versions of ruby/rails you want to support. It keeps your gemspec clean and there's no material difference in terms of functionality.
There was a problem hiding this comment.
I thought the standard was always to put a Gem's dev dependencies in the .gemspec (hence why add_development_dependency exists) then include the .gemspec in the Gemfile like we're currently doing.
Maybe we shouldn't be specifying exact versions for dev dependencies? Would something like Appraisal help with testing against different versions?
There was a problem hiding this comment.
It does look like there's a pending default Rubocop rule for ensuring that development dependencies are enumerated in the Gemfile instead of the gemspec, so I'm happy to move forward with this change.
As @njbbaer mentioned though, Appraisal would be a really nice value add here for testing different Ruby versions. We can follow up on that, though.
|
|
Co-authored-by: Jake Sheehy <[email protected]> Signed-off-by: Ritikesh <[email protected]>
|
@lessthanjacob can we merge this then? |
|
@ritikesh Let's get one more +1, but after that, we should be good to merge! |
|
thanks @lessthanjacob and @jmeridth. I'll post updates on the issue and the fork regarding this as well. |
|
@lessthanjacob @jmeridth - looks like release workflow isn't working? I don't see the 0.3.0 version published to Rubygems post merge |
|
I see the release has happened but not sure if the workflow is fixed. Also, just wanted to know, how do we do communications within the maintainers? |
|
@ritikesh I'll add you to the maintainers mailing list. Yes, release fixed and 0.30.0 is available on rubygems.org. |
Creating this PR to merge the community fork's changes to upstream. I've synced the repos and am creating this PR as-is for now, will cleanup and make changes based on review comments. Have consolidated the contributions into a single changelog entry as well.
Checklist: