(MODULES-3704) Update gemfile template like windows #94
Conversation
|
This is a subset of the work done in #60 |
|
Still WIP, too. |
|
Apart from the rebase...Looks like a good start! |
|
Looks like a good start! |
| end | ||
| end | ||
|
|
||
| <% | ||
| def build_location(gem) |
There was a problem hiding this comment.
Is this still needed or was that changed based on the ruby-operator, ruby-version removals?
There was a problem hiding this comment.
I like that this helped build out the gem information versus continually creating a longer string value.
There was a problem hiding this comment.
But I see you cleaned it up some below from our template and yours, +1!
| @@ -46,29 +46,23 @@ Gemfile: | |||
| - gem: parallel_tests | |||
| - gem: rubocop | |||
| version: '0.41.2' | |||
| ruby-operator: '<' | |||
| ruby-version: '2.0.0' | |||
| condition: "RUBY_VERSION < '2.0.0'" | |||
There was a problem hiding this comment.
I know we don't do it yet, but using Gem::Version would be great for correct semver comparisons.
| ", :ref => '#{gem['ref']}'" if gem['ref'] %><%= | ||
| ", :platforms => #{gem['platforms']}" if gem['platforms'] %><%= | ||
| " if #{gem['condition']}" if gem['condition'] %> | ||
| <% end -%> |
There was a problem hiding this comment.
I like the way that this got cleaned up!
hunner
force-pushed
the
update_gemfile
branch
3 times, most recently
from
f481eb0
to
d4742ef
Compare
hunner
force-pushed
the
update_gemfile
branch
6 times, most recently
from
266a915
to
4647c22
Compare
|
LGTM. Rekicked Travis build. |
hunner
force-pushed
the
update_gemfile
branch
from
4647c22
to
177c8b4
Compare
|
@hunner probably unrelated but travis is failing on bundler: failed to load command: msync (/home/travis/build/puppetlabs/modulesync_configs/vendor/bundle/ruby/2.3.0/bin/msync) |
hunner
force-pushed
the
update_gemfile
branch
8 times, most recently
from
d310998
to
a464082
Compare
hunner
force-pushed
the
update_gemfile
branch
from
a464082
to
41b7e1f
Compare
| - puppetlabs-reboot | ||
| - puppetlabs-registry | ||
| - puppetlabs-sqlserver | ||
| - puppetlabs-wsus_client |
There was a problem hiding this comment.
That's an interesting choice. I would have assumed this was going to be in a different file.
There was a problem hiding this comment.
Shouldn't the supports_windows be in the sync.yml instead of "magic" in modulesync?
There was a problem hiding this comment.
I would prefer supports_windows to go away completely, but this is also in scope for MODULES-4027 (create a set of gems encapsulating our ruby-version-specific dependencies), so I'm supportive of adding a ugly hack here to get this merged.
There was a problem hiding this comment.
@ferventcoder @configs does not expose variables outside of the config_defaults.yml structure under the Gemfile key to the template as far as I saw, so this was the only location without making the template do File.reads, which would be worse...
There was a problem hiding this comment.
@glennsarti Both @ferventcoder and I had a discussion in hipchat yesterday about how .sync.yml should be centralized and go away entirely, but that is a project for another time.
In the mean time, it is modulesync_configs that cares about which modules receive Gemfile template variants, and the modules themselves should be ignorant of "fiddly bits" inside of the templates anyway, so the quality of extra template contents should live with the templates themselves.
There was a problem hiding this comment.
@DavidS Yes! It should go away entirely; see @ferventcoder's and my comment as such at #99 (comment)
hunner
force-pushed
the
update_gemfile
branch
2 times, most recently
from
6e63f9a
to
db885e2
Compare
hunner
force-pushed
the
update_gemfile
branch
from
db885e2
to
265de39
Compare
No description provided.