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.

Sign up for GitHub

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

Jump to bottom

Foreman configuration tags #2413

Merged
merged 3 commits into from Apr 9, 2015
Merged

Foreman configuration tags #2413

merged 3 commits into from Apr 9, 2015

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Mar 28, 2015

Here are some new foreman models.

I added an STI table for basic attributes called ConfigurationTag.

/cc @Fryguy Please let me know if you want to change the model names or have other pieces in mind
/cc @brandondunne @gmcculloug yay more foreman goodness
/cc @gmcculloug please review need automate / tagging

@kbrock kbrock force-pushed the foreman_configuration_tags branch from 22896ad to 51057e9 Compare March 28, 2015 01:40
@kbrock kbrock mentioned this pull request Mar 31, 2015
Merged
@miq-bot
Copy link
Member

miq-bot commented Mar 31, 2015

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@kbrock kbrock added the wip label Mar 31, 2015
@kbrock
Copy link
Member Author

kbrock commented Mar 31, 2015

Going to revisit with an habtm approach instead

@kbrock kbrock force-pushed the foreman_configuration_tags branch from 51057e9 to 21b413a Compare April 1, 2015 13:23
@kbrock kbrock changed the title Foreman configuration tags [WIP] Foreman configuration tags Apr 1, 2015
@kbrock kbrock force-pushed the foreman_configuration_tags branch 2 times, most recently from 1a3c924 to 6411c67 Compare April 2, 2015 13:12
@kbrock kbrock changed the title [WIP] Foreman configuration tags Foreman configuration tags Apr 2, 2015
@kbrock kbrock removed the wip label Apr 2, 2015
@kbrock
Copy link
Member Author

kbrock commented Apr 2, 2015

huh. works for me. Will need to nail this error down.

If anyone gets a chance to review the structure of the classes, please let me know

@kbrock kbrock force-pushed the foreman_configuration_tags branch 2 times, most recently from 46ae5bb to c8a6a64 Compare April 6, 2015 15:47
kbrock
kbrock reviewed Apr 7, 2015
View reviewed changes
vmdb/db/migrate/20150324164033_create_configuration_tags.rb
@@ -0,0 +1,24 @@
class CreateConfigurationTags < ActiveRecord::Migration
def change
create_table :configuration_tags, :id => :bigserial do |t|
Copy link
Member Author

Choose a reason for hiding this comment

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

per @gmcculloug: remove :id => :bigserial

@kbrock kbrock force-pushed the foreman_configuration_tags branch from c8a6a64 to ca11072 Compare April 7, 2015 22:19
gmcculloug
gmcculloug reviewed Apr 8, 2015
View reviewed changes
vmdb/app/models/configuration_tag.rb
acts_as_miq_taggable

belongs_to :manager
end
Copy link
Member

Choose a reason for hiding this comment

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

Should you have has_and_belongs_to_many relationships to point back to configuration_profile and configured_system?

@kbrock kbrock force-pushed the foreman_configuration_tags branch from ca11072 to 877bb2f Compare April 8, 2015 20:58
@kbrock kbrock force-pushed the foreman_configuration_tags branch from 877bb2f to 2357bba Compare April 9, 2015 01:06
@miq-bot
Copy link
Member

miq-bot commented Apr 9, 2015

Checked commits kbrock@8ef1005 .. kbrock@2357bba with rubocop 0.27.1
23 files checked, 2 offenses detected

vmdb/db/migrate/20150324164033_create_configuration_tags.rb

  • 🔹 - Line 2, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for change is too high. [16/15]

vmdb/spec/models/ems_refresh/refreshers/foreman_refresher_spec.rb

  • 🔹 - Line 112, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for assert_configuration_tags is too high. [25/15]

@kbrock kbrock mentioned this pull request Apr 9, 2015
Merged
@gmcculloug
Copy link
Member

LGTM @brandondunne Any comments?

@bdunne
Copy link
Member

bdunne commented Apr 9, 2015

👍 Looks Good

gmcculloug added a commit that referenced this pull request Apr 9, 2015
@gmcculloug
Merge pull request #2413 from kbrock/foreman_configuration_tags
9928ec1 
Foreman configuration tags
@gmcculloug gmcculloug merged commit 9928ec1 into ManageIQ:master Apr 9, 2015
@gmcculloug gmcculloug added this to the Sprint 22 Ending Apr 20, 2015 milestone Apr 9, 2015
@kbrock kbrock deleted the foreman_configuration_tags branch April 9, 2015 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement lifecycle/provisioning providers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kbrock @miq-bot @gmcculloug @bdunne @chessbyte