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

Sk rename ez crypto gem #1

Merged
merged 15 commits into from
May 31, 2019
Merged

Sk rename ez crypto gem #1

merged 15 commits into from
May 31, 2019

Conversation

skapfer
Copy link

@skapfer skapfer commented May 31, 2019

No description provided.

@skapfer
Copy link
Author

skapfer commented May 31, 2019

1 similar comment
@skapfer
Copy link
Author

skapfer commented May 31, 2019

Copy link

@spjsschl spjsschl left a comment

Choose a reason for hiding this comment

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

looks pretty good. Couple of things:

  1. make sure to add a .ruby-version
  2. you do no longer need to appreiase ruby 2.3.3; you can remove it from Rakefile and Appraisals
  3. we usually have a gemfiles directory which stores the gemfiles generated by appraisal for the different versions under test
  4. you should add a cii build

Rakefile Outdated
end

namespace :test do
AfGems::RubyAppraisalTask.new(:all, [ 'ruby-2.3.3', 'ruby-2.5.3'])

Choose a reason for hiding this comment

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

it would be nice if that could appraise 2.5.3 and 2.6.3, you might need to change the Appraisal file as well

Copy link
Author

Choose a reason for hiding this comment

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

Done

Appraisals Outdated
@@ -1,5 +1,5 @@
case(RUBY_VERSION)
when '2.3.3', '2.5.3' then
when '2.3.3', '2.5.3', '2.6.3' then

Choose a reason for hiding this comment

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

you can drop 2.3.3 here and in the Rakefile

Copy link
Author

Choose a reason for hiding this comment

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

Done

ezcrypto.gemspec Outdated
s.version = '0.7.2'
s.date = '2009-03-10'
s.version = EzCrypto::VERSION
s.date = '2019-05-31'

Choose a reason for hiding this comment

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

is this usually hardcoded in our other gems?

Copy link
Author

Choose a reason for hiding this comment

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

I could not get gem bump to work without a version.rb file.
af_crypto, af_encrypted_attr, af_email all have version.rb.

Choose a reason for hiding this comment

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

yep. That is a requirement for AfGem pretty much. I was talking about the date though :-)

Copy link
Author

Choose a reason for hiding this comment

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

Oh. I removed the date, it's pointless

@skapfer skapfer force-pushed the skRenameEzCryptoGem branch 2 times, most recently from c84f3ac to 1f18792 Compare May 31, 2019 13:01
@skapfer skapfer force-pushed the skRenameEzCryptoGem branch from 1f18792 to 3953a13 Compare May 31, 2019 13:05
@skapfer skapfer merged commit a7a6816 into master May 31, 2019
@skapfer skapfer deleted the skRenameEzCryptoGem branch May 31, 2019 13:38
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.

2 participants