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

Generate templates with an option to preserve existing changes #75

Merged
merged 8 commits into from
Dec 16, 2015

Conversation

ghost
Copy link

@ghost ghost commented Dec 8, 2015

changes to config:generate_all so it allows user the option to review the diff and preserve any project specific customization.

@@ -109,9 +109,18 @@ directory are provided:
Run the following command to generate them:

```
[bundle exec] rake config:generate_all
[bundle exec] rake config:generate_all
Copy link
Contributor

Choose a reason for hiding this comment

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

extra space

@ghost
Copy link
Author

ghost commented Dec 9, 2015

@jcarres-mdsol im thinking maybe its cleaner if I create separate 'force' generate tasks

config:generate_all:force
config:generate:force[filename, location]

This will get rid of the additional boolean input. any thoughts?

@jcarres-mdsol
Copy link
Contributor

@atierney-mdsol on one hand sounds good, easier to call. On the other hand we already have a bunch of tasks, I think it will be confusing for users to have too many tasks to choose from and pollute the rails tasks.
Looking at the tasks file in general what's easier for you as user?

@atierney-mdsol
Copy link

@atrivedi-mdsol think I got tagged in by mistake 😄

@@ -1,3 +1,8 @@
# 0.10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can pretty much declare 1.0.0 , as we for all purposes we are treating this gem as semver, no point of staying in 0.x versions

@ghost
Copy link
Author

ghost commented Dec 15, 2015

@jcarres-mdsol I did a very extensive poll (i.e. asked 2 people at work) about which task seem easier. they seem to prefer config:generate_all:force over config:generate_all[true]. I agree, personally i think the :force variant reads better and is more explicit about what it does. it also removes the extra code we need to put in to verify user input.

@jcarres-mdsol
Copy link
Contributor

@atrivedi-mdsol The poll seems extensive enough to me.
Your PR is perfect.
Travis seems to have a weird problem though. Maybe is a hiccup, can you do one more commit to make it run tests again?
Maybe you can add ruby 2.2 to the travis.yml file. We need that anyways. Also .ruby_version should probably be ruby 2.0 at least.

@ghost
Copy link
Author

ghost commented Dec 16, 2015

Thanks @jcarres-mdsol. I updated travis.yml and .ruby-version
travis isn't playing nice with ruby 1.9.3 build. any idea what causes this error? (works locally with 1.9.3)

NoMethodError: undefined method `spec' for nil:NilClass
An error occurred while installing dice_bag (1.0.0), and Bundler cannot
continue

Some gem dependency you reckon?

@ghost
Copy link
Author

ghost commented Dec 16, 2015

looks like CI failure is caused by rubygems/bundler#3559

@jcarres-mdsol
Copy link
Contributor

Probably can be solved by updating to latest bundler version before running
the tests.
Something like adding this to the .travis.yml

install:

  • gem uninstall bundler
  • gem install bundler --version '1.11'

On Wed, Dec 16, 2015 at 9:48 PM, Abhi Trivedi [email protected]
wrote:

looks like CI failure is caused by rubygems/bundler#3559
rubygems/bundler#3559


Reply to this email directly or view it on GitHub
#75 (comment).

Jordi Polo Carres | Software Architect | Medidata Solutions Worldwide
http://www.mdsol.com/

@jcarres-mdsol
Copy link
Contributor

Oh, it seems it passed. I'm having problems with Travis and bundler this couple of days, maybe they solved them. Merging! 🎉

jcarres-mdsol added a commit that referenced this pull request Dec 16, 2015
Generate templates with an option to preserve existing changes
@jcarres-mdsol jcarres-mdsol merged commit 7802171 into mdsol:develop Dec 16, 2015
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.

3 participants