-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add CI on GitHub for 1.x-stable branch #2640
Add CI on GitHub for 1.x-stable branch #2640
Conversation
4de53a2
to
4524f79
Compare
b1120c9
to
1cfb2a3
Compare
What prevents you from upgrading to a later version of CarrierWave? I don't recommend people to stay on 1.x, without a good reason. Even if you want to, please don't change Ruby and Rails versions from what's in .travis.yml. 1.x is meant to be compatible with them. |
Hi @mshibuya, I'm chiming in as a colleague of @marcogregorius at GitLab. We are trying to upgrade GitLab to Ruby 3.0. GitLab uses CarrierWave so we want to make sure CarrierWave works with Ruby 3.0. Several years ago we added a series of monkey patches to CarrierWave 1.x in GitLab. You can read about them here. These modifications make it more difficult for us to upgrade to 2.x. Additionally, we have some concerns about different use of ActiveRecord callbacks in CarrierWave 2.x so we may want to wait for CarrierWave 3.x. Because of the large number of Ruby gems GitLab depends on, upgrading to Ruby 3.0 is proving to be a lot of work. We would like to avoid making GitLab's Ruby 3.0 upgrade dependent on resolving our (self-created) CarrierWave challenges. What we would need for that is a 1.x version of CarrierWave that is known to work with Ruby 3. I would completely understand if you do not want to make these kind of changes to the @mshibuya are you open to having a CarrierWave 1.x release with Ruby 3 support? |
Adding Ruby 3.0 support to 1.x-stable is OK, as long as you maintain the compatibility with Ruby >= 2.2 and Rails >= 4.0. I can cut a new gem release for 1.x if you need it. And I have one proposal with your 3.0 migration plan. |
051d55d
to
a089375
Compare
a089375
to
4f1e1d2
Compare
This is a very reasonable question and I appreciate the invitation. I really struggle deciding how to answer this though. We are not happy with the monkey patches but they work. At this time it is not clear which of the challenges we have with our current solution are most important to address. Being stuck on CarrierWave 1.x is just one of the problems. Having said that, I want to say again I appreciate the invitation to work inside the CarrierWave code base instead of around it. |
740488f
to
7ff231b
Compare
7ff231b
to
5977ee0
Compare
Thanks @mshibuya , that would be much appreciated! However, I tried adding Ruby >= 2.2 and Rails >= 4.0 and changed Alternatively, I also tried using the original
|
OK I can handle the rest. Thank you! |
@jacobvosmaer @marcogregorius Can I proceed with creating a new 1.x release, or do you need some more changes? |
@mshibuya Thanks for the release! We have bumped GitLab to use the new version since. Again, thanks for the effort! |
As branch
1.x-stable
doesn't have CI on GitHub, I added thetest
workflow based onmaster
branch.Notable changes:
sqlite3
instead ofpostgres
from f7beff6URI.encode
withURI::DEFAULT_PARSER.escape
for Ruby 3 testsssrf_filter
gem to < 1.1.0 due to the breaking changes at 1.1.0fog-google
requirementruby-head
andjruby-head
asruby-head
fails some tests https://github.com/carrierwaveuploader/carrierwave/actions/runs/3514804110/jobs/5889275676 (not sure why?)