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

Developer bootstrap improvements #161

Merged
merged 47 commits into from Feb 21, 2017

Conversation

purp
Copy link
Contributor

@purp purp commented Feb 15, 2017

Just a rake task to bootstrap a new developer environment.

The sqlite example won’t work in development due to dependencies on
ThinkingSphinx, which doesn’t support sqlite3.

The vagrant example works fine in non-vagrant setups.
Add rake task to bootstrap dev setup.
* Copy config examples in place
* Perform bundle:install
* Check for Sphinx install
* generate Sphinx configs (via ThinkingSphinx)
* generate Sphinx indices
@purp
Copy link
Contributor Author

purp commented Feb 15, 2017

Working on fixing travis-ci. Will update shortly.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.817% when pulling 07c2900 on purp:developer_bootstrap_improvements into 3e1d527 on SUSE:master.

Not needed for travis-ci anymore
Allows `travis lint` for checking travis config
@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.817% when pulling dddf262 on purp:developer_bootstrap_improvements into 3e1d527 on SUSE:master.

purp and others added 4 commits February 15, 2017 13:27
Cleans up deprecation:

DEPRECATION WARNING: You are passing an instance of ActiveRecord::Base
to `find`. Please pass the id of the object by calling `.id`. (called
from load_episode at
/home/travis/build/SUSE/hackweek/app/controllers/projects_controller.rb:
207)
Cleans up deprecation:

DEPRECATION WARNING: Extra .css in SCSS file is unnecessary. Rename
/home/travis/build/SUSE/hackweek/app/assets/stylesheets/strap-on.css.scs
s to
/home/travis/build/SUSE/hackweek/app/assets/stylesheets/strap-on.scss.
(called from
_app_views_projects_show_html_haml___2150580354090734029_98454160 at
/home/travis/build/SUSE/hackweek/app/views/projects/show.html.haml:16)
DEPRECATION WARNING: Extra .css in SCSS file is unnecessary. Rename
/home/travis/build/SUSE/hackweek/app/assets/stylesheets/zoomed.css.scss
to /home/travis/build/SUSE/hackweek/app/assets/stylesheets/zoomed.scss.
(called from
_app_views_projects_show_html_haml___2150580354090734029_98454160 at
/home/travis/build/SUSE/hackweek/app/views/projects/show.html.haml:16)
DEPRECATION WARNING: Extra .css in SCSS file is unnecessary. Rename
/home/travis/build/SUSE/hackweek/app/assets/stylesheets/strap-on.css.scs
s to
/home/travis/build/SUSE/hackweek/app/assets/stylesheets/strap-on.scss.
(called from
_app_views_projects_show_html_haml___2150580354090734029_98454160 at
/home/travis/build/SUSE/hackweek/app/views/projects/show.html.haml:16)
DEPRECATION WARNING: Extra .css in SCSS file is unnecessary. Rename
/home/travis/build/SUSE/hackweek/app/assets/stylesheets/zoomed.css.scss
to /home/travis/build/SUSE/hackweek/app/assets/stylesheets/zoomed.scss.
(called from
_app_views_projects_show_html_haml___2150580354090734029_98454160 at
/home/travis/build/SUSE/hackweek/app/views/projects/show.html.haml:16)
Gets all the rest of these:

DEPRECATION WARNING: You are passing an instance of ActiveRecord::Base
to `find`. Please pass the id of the object by calling `.id`. (called
from load_episode at /Users/purp/Library/Mobile
Documents/com~apple~CloudDocs/purp/work/hackweek/app/controllers/project
s_controller.rb:207)
.travis.yml Outdated
@@ -10,10 +12,13 @@ notifications:
on_failure: change
before_install:
- "echo 'gem: --no-ri --no-rdoc' > ~/.gemrc"
before_script:
- mysql -e 'CREATE DATABASE hackweek_test;'
Copy link
Member

Choose a reason for hiding this comment

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

bundle exec rake db:create

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree that should work. I was following the recommendation from the Travis CI docs for MySQL with ActiveRecord. Will change to bundle exec

.travis.yml Outdated
- RAILS_ENV=test bundle exec rake db:migrate --trace
- RAILS_ENV=test bundle exec rake db:migrate --trace
env:
- TRAVIS=true
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! I missed that, and "invented" my own. Will change.

Gemfile Outdated
@@ -73,6 +72,8 @@ gem 'capybara', :group => [:development, :test]
gem 'paperclip', '~> 4.1'
# as interactive debugger in error pages
gem 'web-console', '~> 2.0', group: :development
# Help with travis config testing
gem 'travis', :group => [:development, :test]
Copy link
Member

Choose a reason for hiding this comment

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

Too many dependencies just for a linter, we don't have to have this in the bundle I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair. I'll drop it.

@@ -0,0 +1,51 @@
# Don't add any includes that aren't in the Ruby std-lib here,
Copy link
Member

Choose a reason for hiding this comment

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

We already have the vagrant box and this is duplicating bootstrap.sh a lot. Too much for my taste, sorry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand, and I'm not sure I see the duplication that you do.

The rake task intends to handle only Rails-oriented environment setup. It does five main things:

  1. Copy required Rails config files into place
  2. Install required Ruby gems using the bundle command
  3. Set up the required databases for the Rails app (create, migrate, and seed)
  4. Generate Sphinx configuration for the Rails app (first checking that Sphinx is installed)
  5. Generate Sphinx indexes for the Rails app

When I look at bootstrap.sh, the only overlap I see between this rake task and bootstrap.sh is copying the proper database configuration file in place. Perhaps I'm not seeing other parts which overlap. Could you say more about this?

Meanwhile, I think we could separate concerns into two here:

  1. Setting up the development machine (installing packages like mysql and sphinx, configuring the system, etc.) This should definitely be handled by bootstrap.sh for the OpenSUSE vagrant-based development environment.
  2. Setting up the Rails app and Rails environment. This could be handled for all environments by the rake task here.

Doing this could DRY up a couple of things:

  • bootstrap.sh could call the rake task once the development machine setup is complete
  • We could replace the entire before_script section in travis.yml with bundle exec rake dev:bootstrap

I'm happy to extend this PR to cover those changes, as well as updating the README, if you agree with this direction.

Copy link
Member

Choose a reason for hiding this comment

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

That would be a way to make setting up in travis/vagrant more DRY yes.

However I'm not really convinced that we need to 'hide' away the standard rails env setup tasks from people. People are used to this, this is mentioned in documentation, tutorials etc. etc.

Let's do this for travis/vagrant and only mention this after the 'standard' steps like:

To make this more convenient for you we have setup a rake task that executes the above steps:

bundle exec rake dev:boostrap

Okay? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it. I'll update the PR this morning.

As @hennevogel points out, this is a default set by Travis CI.
It adds a kerjillion dependencies.
`rake` is evidently not available on the system in `before_install`
@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.817% when pulling 18a5940 on purp:developer_bootstrap_improvements into 3e1d527 on SUSE:master.

@purp
Copy link
Contributor Author

purp commented Feb 17, 2017

That leads to an interesting failure that I can't reproduce locally. I'm going to ponder how to debug this a bit.

mdeniz and others added 2 commits February 18, 2017 00:58
* Rails.root.join was overcomplicated
* I forgot to include the actual copy command =\
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 78.626% when pulling a0d1b05 on purp:developer_bootstrap_improvements into 3e1d527 on SUSE:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 78.626% when pulling a319633 on purp:developer_bootstrap_improvements into 3e1d527 on SUSE:master.

@hennevogel
Copy link
Member

Fixed in 031482c

@hennevogel
Copy link
Member

@purp anything left to do?

hennevogel and others added 19 commits February 20, 2017 14:14
The sqlite example won’t work in development due to dependencies on
ThinkingSphinx, which doesn’t support sqlite3.

The vagrant example works fine in non-vagrant setups.
Add rake task to bootstrap dev setup.
* Copy config examples in place
* Perform bundle:install
* Check for Sphinx install
* generate Sphinx configs (via ThinkingSphinx)
* generate Sphinx indices
Not needed for travis-ci anymore
Allows `travis lint` for checking travis config
As @hennevogel points out, this is a default set by Travis CI.
It adds a kerjillion dependencies.
`rake` is evidently not available on the system in `before_install`
* Rails.root.join was overcomplicated
* I forgot to include the actual copy command =\
@purp
Copy link
Contributor Author

purp commented Feb 20, 2017

Updating to catch up with upstream master, then I'll see if your fix solves the vagrant filesystem mounting issue I saw. If so, I'll finish up changes to bootstrap.sh.

Incidentally, the filesystem was mounting when the VM started; I was able to verify via vagrant exec ls -l /vagrant while bootstrap was running. I was able to narrow it to before/after the zypper dup operation.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.817% when pulling 9d0e352 on purp:developer_bootstrap_improvements into 3e1d527 on SUSE:master.

@purp
Copy link
Contributor Author

purp commented Feb 20, 2017

Okay, testing seems to show that this works for bootstrapping as well. If it passes, I'm all done here.

Thanks, @hennevogel!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.817% when pulling 3566e99 on purp:developer_bootstrap_improvements into 3e1d527 on SUSE:master.

@hennevogel
Copy link
Member

Thank you @purp for your awesome contribution 💐 🎆 🍀

@hennevogel hennevogel merged commit a0d25b0 into SUSE:master Feb 21, 2017
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

3 participants