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

generator for providers #12209

Merged
merged 3 commits into from
Dec 8, 2016
Merged

Conversation

durandom
Copy link
Member

@durandom durandom commented Oct 26, 2016

creates an empty provider at providers/<provider_name>
use as rails generate provider UberCloud

@miq-bot add_labels pluggable providers
@miq-bot assign Fryguy

cc @juliancheal @blomquisg @Ladas

@juliancheal
Copy link
Member

juliancheal commented Oct 26, 2016

Oh poor @miq-bot. Is this the first template we've created?

Can't see any cops we can add to Rubocop to validate erb in .rb files.

@Fryguy
Copy link
Member

Fryguy commented Oct 26, 2016

We can add this whole directory to the excludes list, probably.

@Fryguy
Copy link
Member

Fryguy commented Oct 26, 2016

Also 😮 😮 😮 😮 Very excited!!

@@ -0,0 +1,59 @@
require "rails/generators/rails/app/app_generator"
class ProviderGenerator < Rails::Generators::NamedBase
source_root File.expand_path('../templates', __FILE__)
Copy link
Member

Choose a reason for hiding this comment

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

Prefer File.expand_path("templates", __dir__)

@@ -0,0 +1,2 @@
class ManageIQ::Providers::<% class_name %>::CloudManager < ManageIQ::Providers::CloudManager
Copy link
Member

Choose a reason for hiding this comment

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

Should this be <%= vs <%

APP_ROOT = Pathname.new File.expand_path('../../', __FILE__)

Dir.chdir APP_ROOT do
if File.exists?("spec/manageiq")
Copy link
Member

Choose a reason for hiding this comment

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

Rubocop: File.exists? is deprecated in favor of File.exist?.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pasting CodeClimate? 😛 But thanks.

export BUNDLE_WITHOUT=development
export BUNDLE_GEMFILE=${PWD}/Gemfile

set +v
Copy link
Member

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 we could keep super-common stuff like this in the ManageIQ repo, and then refer to it somehow. Like, perhaps keep the git clone and then have the rest of the file live in ManageIQ/tools/ci somewhere?

The more that's duplicated across provider repos, the more it has to be fixed across the board.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed. I'll see what I can do.

OTOH, whats nice about the generator: you can run it again and have it patch the files in the repos

[![Code Climate](https://codeclimate.com/github/ManageIQ/manageiq-providers-<%= provider_name %>.svg)](https://codeclimate.com/github/ManageIQ/manageiq-providers-<%= provider_name %>)
[![Test Coverage](https://codeclimate.com/github/ManageIQ/manageiq-providers-<%= provider_name %>/badges/coverage.svg)](https://codeclimate.com/github/ManageIQ/manageiq-providers-<%= provider_name %>/coverage)
[![Dependency Status](https://gemnasium.com/ManageIQ/manageiq-providers-<%= provider_name %>.svg)](https://gemnasium.com/ManageIQ/manageiq-providers-<%= provider_name %>)
[![Security](https://hakiri.io/github/ManageIQ/manageiq-providers-<%= provider_name %>/master.svg)](https://hakiri.io/github/ManageIQ/manageiq-providers-<%= provider_name %>/master)
Copy link
Member

@Fryguy Fryguy Oct 26, 2016

Choose a reason for hiding this comment

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

Where's the zanata badge.

Copy link
Member Author

Choose a reason for hiding this comment

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

there it is now. 👇

@Fryguy
Copy link
Member

Fryguy commented Oct 26, 2016

creates an empty provider at providers/<provider_name>

Wonder if it should create it at ../manageiq-providers-provider_name (and then also git init in that directory if possible.

@Fryguy
Copy link
Member

Fryguy commented Oct 26, 2016

This is really great stuff @durandom !!

notifications:
webhooks:
urls:
- https://webhooks.gitter.im/e/7af45001fe6b7a4039f2
Copy link
Member

Choose a reason for hiding this comment

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

This is a webhook for a specific gitter channel, so this should not be in the template.

@djberg96
Copy link
Contributor

@durandom Wouldn't it be easier to develop and maintain this as a rails plugin in its own repo rather than baked into ManageIQ directly?

@Fryguy
Copy link
Member

Fryguy commented Oct 27, 2016

I'm ok with it starting here and extracting afterwards.

@durandom
Copy link
Member Author

@djberg96 I totally agree with you, but in order to get this jumpstarted I implemented it here first. This gives us CI and other stuff. I also need it quite soon to extract the other providers.

If you want to help out on doing this you are more than welcome 😄

@durandom durandom force-pushed the provider_generator branch 3 times, most recently from 9e2e920 to 5d59d3e Compare October 28, 2016 12:17
@durandom
Copy link
Member Author

@jrafanie can you have a look at the split of tools/ci/ scripts here? This is done to call those bits as needed from a provider repo.

If ok, I can extract them to their own PR.

@durandom
Copy link
Member Author

ManageIQ/manageiq-providers-amazon#71 applies the output to the manageiq repo and CI is still working there

cp config/database.pg.yml config/database.yml
cp config/cable.yml.sample config/cable.yml
psql -c "CREATE USER root SUPERUSER PASSWORD 'smartvm';" -U postgres

Copy link
Member

Choose a reason for hiding this comment

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

Looks good @durandom 👍

If these changes work, it's good to bucket these various setups into categories.

@jrafanie
Copy link
Member

@jrafanie can you have a look at the split of tools/ci/ scripts here? This is done to call those bits as needed from a provider repo.

If ok, I can extract them to their own PR.

👍 I like it. My only concern that seems to keep creeping in is ways to keep your copied templates in sync with any changes we make in MIQ. We probably need some ways to keep repos in sync since we'll be routinely copying content to new repos from old locations.

@jrafanie
Copy link
Member

@durandom this PR is solid "Like a rock, like a planet"

@durandom
Copy link
Member Author

Like a rock, like a planet

like a f*** 💣

@durandom durandom force-pushed the provider_generator branch 5 times, most recently from 8c9cdf5 to a78859c Compare November 9, 2016 10:33
@durandom
Copy link
Member Author

durandom commented Nov 9, 2016

@Fryguy its green, merge?

@durandom durandom force-pushed the provider_generator branch 2 times, most recently from d17de54 to 55c6209 Compare December 7, 2016 15:20
creates an empty provider at providers/<provider_name>
use as rails generate provider
@miq-bot
Copy link
Member

miq-bot commented Dec 7, 2016

<github_pr_commenter_batch />Some comments on commits durandom/manageiq@4a72236~...141d49b

lib/generators/provider/templates/Rakefile

  • ⚠️ - 4 - Detected puts. Remove all debugging statements.

lib/generators/provider/templates/bin/setup

  • ⚠️ - 12 - Detected puts. Remove all debugging statements.
  • ⚠️ - 16 - Detected puts. Remove all debugging statements.
  • ⚠️ - 21 - Detected puts. Remove all debugging statements.
  • ⚠️ - 26 - Detected puts. Remove all debugging statements.
  • ⚠️ - 9 - Detected puts. Remove all debugging statements.

@miq-bot
Copy link
Member

miq-bot commented Dec 7, 2016

Checked commits durandom/manageiq@4a72236~...141d49b with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 3 offenses detected

lib/generators/provider/provider_generator.rb

@durandom
Copy link
Member Author

durandom commented Dec 7, 2016

@jrafanie do you dare to merge this? 😈

@jrafanie
Copy link
Member

jrafanie commented Dec 8, 2016

While I like the templates, I wonder if the generator could fetch the latest versions of the templates from the main repo, where the templates in this PR are identical with the main repos... Either way, we can do that later. I've seen it all along, LGTM.

@jrafanie jrafanie merged commit 744abc5 into ManageIQ:master Dec 8, 2016
@jrafanie jrafanie added this to the Sprint 51 Ending Jan 2, 2017 milestone Dec 8, 2016
@durandom
Copy link
Member Author

durandom commented Dec 8, 2016

@jrafanie thanks for merging 🎉

Will try to address your concerns / ideas in follow ups

@durandom durandom deleted the provider_generator branch December 8, 2016 21:38
@durandom durandom mentioned this pull request Apr 21, 2017
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants