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

Using fog-aws instead of fog as dependency #295

Closed
wants to merge 1 commit into from

Conversation

eljojo
Copy link

@eljojo eljojo commented Feb 10, 2015

Please don't merge this yet. I'm opening the PR so travis tests it :)

As discussed in #292, this is a possible solution to use fog-aws instead of fog.
As fog-aws is fairily new, there are a couple of problems:

  • probably users of old versions of fog would have to update to at least 1.27. That shouldn't be a problem, though, because fog follows semantic versioning
  • I'm not sure of what's an appropiate version to use as a minimum version for fog-aws so I just chose the most recent one.

@eljojo
Copy link
Author

eljojo commented Feb 10, 2015

I just deployed using my branch and it all worked fine 💃

       Installing fog-core 1.28.0
       Using fog-xml 0.1.1
       Installing fog-aws 0.1.0
       Using asset_sync 1.1.0 from git://github.com/eljojo/asset_sync.git (at fog_aws)
       Your bundle is complete!
       Gems in the groups development and test were not installed.
       It was installed into ./vendor/bundle
       Bundle completed (79.74s)
       Cleaning up the bundler cache.
       Removing fog-core (1.24.0)
       Removing fog-softlayer (0.3.24)
       Removing fog-brightbox (0.6.1)
       Removing fog (1.24.0)
       Removing asset_sync (1.1.0)
       Removing net-ssh (2.9.1)
       Removing fog-radosgw (0.0.3)
       Removing fog-sakuracloud (0.1.1)
       Removing excon (0.42.1)
       Removing inflecto (0.0.2)
-----> Preparing app for Rails asset pipeline
       Running: rake assets:precompile
       AssetSync: using /tmp/build/config/initializers/asset_sync.rb
       AssetSync: Syncing.
       Using: Directory Search of /tmp/build/public/assets
       AssetSync: Done.
       Asset precompilation completed (8.87s)
       Cleaning assets
       Running: rake assets:clean
       AssetSync: using /tmp/build/config/initializers/asset_sync.rb

@j15e
Copy link

j15e commented Mar 30, 2015

+1 for this change, when you install the latest full "fog" gem it depends on like 15 gems for each provider, when we actually only need one.

@eljojo
Copy link
Author

eljojo commented Mar 30, 2015

ping @davidjrice @rumblelabs?

@j15e j15e mentioned this pull request Apr 15, 2015
@Andrekra
Copy link

+1 Asset sync is made specifically for S3 (according to the docs) So no need to include the fog gem which loads:

     fog-atmos
      fog-aws
      fog-brightbox
      fog-core
      fog-ecloud
      fog-google
      fog-json
      fog-local
      fog-powerdns
      fog-profitbricks
      fog-radosgw
      fog-riakcs
      fog-sakuracloud
      fog-serverlove
      fog-softlayer
      fog-storm_on_demand
      fog-terremark
      fog-vmfusion
      fog-voxel
      fog-xml

@josacar
Copy link
Contributor

josacar commented Jul 24, 2015

@davidjrice Any update on this?

@urkle
Copy link

urkle commented Aug 20, 2015

Please can we get this into a new asset_sync release? We are trying to reduce gem bloat, and pulling in all of the fog is excessive bloat.

@Chocksy
Copy link

Chocksy commented Aug 20, 2015

@rumblelabs any idea if we can get this merged? It seems to not break anything and a simple change. Also it's super obvious we don't need the whole fog dependencies in here. We tried to use the fog loading before the asset_sync but it doesn't seem to work.

@dgilperez
Copy link

We are very needing this.

@coneybeare
Copy link

Please merge.

@lime
Copy link
Collaborator

lime commented Aug 25, 2016

This indeed seems like a good improvement to me. 👍

I'd like to give this a try locally before merging, especially since our Travis build is out of shape at the moment. Sadly I won't have any time at all this week. I'll give it a look next week, but feel free to ping me if that doesn't happen. :)

@PikachuEXE
Copy link
Member

I am only a new member so I will let the project owner decide.
Also I prefer #325 instead of this PR, since that's more generic solution.
But both PR seem to be a "breaking" change. Hence I am unable to decide.

@PikachuEXE
Copy link
Member

2.0.0 released
Have a try!

@eljojo eljojo closed this Mar 22, 2017
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.

10 participants