Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tech 4449 appraisal phase 1 #19

Merged
merged 11 commits into from
May 15, 2020
Merged

Tech 4449 appraisal phase 1 #19

merged 11 commits into from
May 15, 2020

Conversation

jebentier
Copy link
Collaborator

[0.7.0] - Unreleased

Added

  • Added support for rails 5 and 6.
  • Added appraisal tests for all supported rails version: 4/5/6

Changed

  • Updated various test to be compatible with rails version 4/5/6
  • Updated the CI pipeline to test against all three supported versions of rails

Fixed

  • Fixed undefined method delegate bug in ActiveSupport version 4

@jebentier jebentier self-assigned this May 12, 2020
Copy link
Contributor

@ColinDKelley ColinDKelley left a comment

Choose a reason for hiding this comment

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

Looks good!

I have a couple questions and comments.

version_requirement_updates: "off"
commit_message:
prefix: "No-Jira"
include_scope: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing newline here I think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

looks like my script isn't adding the newline to the end 👎 i've gone ahead and added it

@@ -51,3 +51,6 @@ build-iPhoneSimulator/

# rubocop
.rubocop-https---raw-githubusercontent-com-Invoca-style-guide-master-ruby--rubocop-yml

# appraisal
gemfiles/*.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting--I'd missed this subtlety. The appraisals generate a fresh Gemfile.lock each time?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea, this is a best practice set by the appraisals gem to make sure that the appraisal is always tested against the most recent version of the lock. it leans very heavily on semantic versioning and locking.

@@ -0,0 +1 @@
2.6.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the hyphen file best practice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

.ruby_version isn't picked up by rbenv only .ruby-version is. so the file name change was necessary for the configuration to take affect.


appraise 'rails-6' do
gem 'activesupport', '~> 6.0'
end
Copy link
Contributor

Choose a reason for hiding this comment

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

💯 So clean!

gem 'rspec_junit_formatter'
end
gem 'rspec'
gem 'rspec_junit_formatter'
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for simplifying here!

CHANGELOG.md Outdated
@@ -3,6 +3,18 @@
Inspired by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
Note: this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.7.0] - Unreleased
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW I have 0.7.0 also in a branch. We may be able to release those together.

@logger.formatter = ->(_, _, _, msg_with_context) { "#{msg_with_context.to_json}\n" }
@logger = ActiveSupport::TaggedLogging.new(@logger)
@logger = ContextualLogger.new(@logger)
Copy link
Contributor

Choose a reason for hiding this comment

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

Yuck, I hadn't noticed how this overwrites @logger! I will fix that on my branch.

@jebentier jebentier merged commit 1df9252 into master May 15, 2020
@jebentier jebentier deleted the TECH-4449_appraisal_phase_1 branch May 15, 2020 16:41
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.

None yet

2 participants