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

refs #3752 - run db:seed after DB migration #127

Closed
wants to merge 1 commit into from

Conversation

domcleal
Copy link
Contributor

Using foreman-rake means 1.3+, but I think we said that we'd support current stable + the development version. Running db:seed on 1.3 will be fine as either the seed file doesn't exist (no error) or it contains next to nothing.

@ekohl
Copy link
Member

ekohl commented Nov 27, 2013

I'm starting to think about a foreman::rake define which executes foreman-rake to remove some of the duplication, but I have no problem merging this as is.

@domcleal
Copy link
Contributor Author

@ekohl how's that?

@@ -0,0 +1,10 @@
# Run a Foreman rake task when notified
define foreman::rake() {
exec { "rake-${title}":
Copy link
Member

Choose a reason for hiding this comment

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

Maybe name this foreman-rake-${title}?

@ekohl
Copy link
Member

ekohl commented Nov 27, 2013

@domcleal Minor comments inline, but I like it.

@domcleal
Copy link
Contributor Author

Thanks for the feedback, PR updated.

@ekohl
Copy link
Member

ekohl commented Nov 27, 2013

👍 looks great.

@domcleal
Copy link
Contributor Author

Please hold on merging until the core PR hits and is merged.

@ekohl
Copy link
Member

ekohl commented Nov 27, 2013

It should still work on released versions so what is the problem with nightly?

@domcleal
Copy link
Contributor Author

@ekohl just in case my PR's rejected :)

@ekohl
Copy link
Member

ekohl commented Nov 27, 2013

@domcleal fair enough, but then I still like foreman::rake as an abstraction for foreman-rake.

# Run a Foreman rake task when notified
define foreman::rake() {
exec { "foreman-rake-${title}":
command => "/usr/sbin/foreman-rake ${title}",
Copy link
Member

Choose a reason for hiding this comment

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

Is foreman-rake in sbin on Debian. I think @GregSutcliffe mentioned something about hate of "sbins" :-D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is:

debian/precise/foreman/foreman.install
5:script/foreman-debug usr/sbin
6:script/foreman-rake usr/sbin

Copy link
Member

Choose a reason for hiding this comment

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

I may not like them, but that's because on Arch, everything is in /usr/bin :P - on Debian, I still have to use their guidelines...

@GregSutcliffe
Copy link
Member

👍 as and when core is updated. also 👍 to keeping foreman::rake if it isn't :)

@domcleal
Copy link
Contributor Author

Core PR's merged, this is now needed for nightlies, would somebody mind merging please?

@ekohl
Copy link
Member

ekohl commented Dec 25, 2013

Merged in 9344250.

@ekohl ekohl closed this Dec 25, 2013
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.

None yet

4 participants