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

Swap app and gem #144

Merged
merged 1 commit into from Nov 5, 2015
Merged

Swap app and gem #144

merged 1 commit into from Nov 5, 2015

Conversation

danbee
Copy link
Contributor

@danbee danbee commented Nov 4, 2015

Moves the gem to the root directory and moves the example app to a
directory within the /spec directory. This makes it possible to refer
to the gem directly from git or Github.

TODO

  • Restore Appraisal runs in the integration test suite
  • Figure out how to deploy demo app to Heroku

@c-lliope
Copy link
Contributor

c-lliope commented Nov 4, 2015

Looks like the build isn't running the appraisal builds anymore. I'm gonna start a checklist in the description.

@danbee
Copy link
Contributor Author

danbee commented Nov 4, 2015

Yeah, I'm sure there's a boatload of stuff that I've missed/broken.

@c-lliope
Copy link
Contributor

c-lliope commented Nov 4, 2015

but the specs are passing now - that's cool. Before the rebase, I was getting failures for missing translations.

@danbee
Copy link
Contributor Author

danbee commented Nov 4, 2015

Ah no, that was fixed separately too. The i18n-tasks configuration was in the wrong place and I had to rename the translate method in the application_controller.rb file as I think it was getting overridden by i18n's built in one.

config.before(js: true) do
page.driver.block_unknown_urls
end
Capybara::Webkit.configure do |c|

Choose a reason for hiding this comment

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

Pass &:block_unknown_urls as an argument to configure instead of a block.

@danbee
Copy link
Contributor Author

danbee commented Nov 4, 2015

CircleCI is now running Appraisals.

@c-lliope
Copy link
Contributor

c-lliope commented Nov 4, 2015

🎉

@c-lliope
Copy link
Contributor

c-lliope commented Nov 5, 2015

@danbee this looks great! Now, what color should we paint the bikeshed? spec/demo or spec/example_app?

I was initially thinking demo because I've been calling it "the demo site" in most of the copy/documentation. example_app's a bit more explicit though, which is nice. Thoughts?

@danbee
Copy link
Contributor Author

danbee commented Nov 5, 2015

A lot of the docs/articles I read called it spec/dummy_app which I didn't think was too appropriate given the nature of it. Hence why I chose example_app.

spec/demo_app could work though.

@jayroh
Copy link
Collaborator

jayroh commented Nov 5, 2015

This is probably completely up to the project but the convention I've seen is to house this in spec/dummy. That being said. That pattern is usually in cases where it's used against the specs/tests exclusively.

So ... ¯_(ツ)_/¯

@JoelQ
Copy link
Contributor

JoelQ commented Nov 5, 2015

FWIW, I believe that the generator for Rails engines creates spec/dummy by default.

@danbee
Copy link
Contributor Author

danbee commented Nov 5, 2015

@JoelQ Indeed it does! I created a test engine to compare to.

@c-lliope
Copy link
Contributor

c-lliope commented Nov 5, 2015

yeah, I'm against dummy. I'd vote for example_app or demo_app, both are good.

@danbee
Copy link
Contributor Author

danbee commented Nov 5, 2015

I wondered about whether it should even live in the spec directory to be honest.

@JoelQ
Copy link
Contributor

JoelQ commented Nov 5, 2015

Should there be a separate demo app (possibly in it's own repo) that gets deployed to heroku and a spec/dummy to use in feature specs?

@c-lliope
Copy link
Contributor

c-lliope commented Nov 5, 2015

@JoelQ yeah, we'll probably want to break that out eventually. Right now there's test coupling, so it would be a bit painful in the near term.

desc "Deploy the example app to Heroku"
namespace :deploy do
task :example do
exec 'git push heroku `git subtree split --prefix spec/example_app`:master --force'
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this heroku remote differs from what gets set up in bin/setup? Can we change bin/setup to add contributors to the correct heroku app, with a remote name that matches this?

Also, I've been preferring the $(shell_command) format, because:

  1. It differentiates the start and end of a command
  2. It's copy-and-pastable into Slack code snippets 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, then I guess we need two deploy tasks?

@c-lliope
Copy link
Contributor

c-lliope commented Nov 5, 2015

@danbee one inline comment, but then this looks good to me!

Were there any outstanding tasks I forgot about?

@danbee
Copy link
Contributor Author

danbee commented Nov 5, 2015

I don't think so!

@c-lliope
Copy link
Contributor

c-lliope commented Nov 5, 2015

Alright. I'm going to add a CHANGELOG entry (the only user-facing change is the ability to point bundler to the gem on github) and merge it. Thanks! 🎉

@c-lliope
Copy link
Contributor

c-lliope commented Nov 5, 2015

I just ran gem build administrate.gemspec and then tested the built gem locally - everything still works!

Problem:

Because `administrate.gemspec` was not at the root of the repository,
users could not install the gem by pointing their Gemfile at Github.

Solution:

Move the gem to the root directory and moves the example app to a
directory within the `/spec` directory. This makes it possible to refer
to the gem directly from git or Github.

The example app is stored as a [git subtree], which means it can be
deployed separately to Heroku or other hosting services.

[git subtree]: https://blogs.atlassian.com/2013/05/alternatives-to-git-submodule-git-subtree/

Change notes:

- The `translate` method defined in the controller was causing i18n-tasks
to get confused about the missing keys in the locale file. Renamed it to
`translate_with_resource`.

- Fix Capybara Webkit deprecation warning
- Load schema before running specs
- Update the example apps Gemfile to point at Github, so the example app
  can be deployed to Heroku. For local development, the local copy of
  `administrate` takes precedence over the example app's Gemfile, and
  tests still run as expected.
- Add rake task to deploy example app. This uses
  more-complex-than-normal git subtree commands, so it's good to have a
  shortcut.
@danbee
Copy link
Contributor Author

danbee commented Nov 5, 2015

I'm making a note here: Huge success!

@c-lliope c-lliope merged commit 01e125c into master Nov 5, 2015
@c-lliope c-lliope deleted the swap-gem-and-test-app branch November 5, 2015 18:00
@c-lliope
Copy link
Contributor

c-lliope commented Nov 5, 2015

c-lliope added a commit that referenced this pull request Nov 6, 2015
Problem:
--------

With the git subtree strategy adopted in #144,
there are several unconventional things
surrounding the heroku deploy process:

- Pushing to heroku requires a `git subtree` command
- We need special logic in the Gemfile in order to run the demo app
  locally with our local version of Administrate.
- `rake` and `rails` commands need to be run from the
  `spec/example_app` subdirectory.

In addition, there is a dependency problem between the example app
and the master branch of administrate on github.
The example app needs to pull the master branch
as a dependency on heroku,
but the `Gemfile.lock` locks it in to a specific version.

It is impossible to point the `Gemfile.lock`
at the repo's current commit,
because that would require a new commit
before we could deploy to Heroku.

The result would be that the branch on Heroku would always differ
from the branch on GitHub by at least one commit,
and we would need to develop special tooling around pushing to heroku.

Solution:
---------

Make the `rails`, `rake`, and `bundle exec` commands work
from the root directory of the repository.
This involved:

- Move the `config.ru` script to the root of the repo
- Update `rake` and `rails` binstubs to reference nested rails app
- Move `unicorn.rb` (required by Procfile) to top-level `config` dir
- Combine both Gemfiles into a single one, at the root of the repo
- Remove spring for simplicity

I tested the following tasks.
All work from the root directory of the repo:

- deploy to heroku: `git push heroku master`
- run server locally: `rails server` or `foreman start`
- seed demo database: `rake db:seed`
- migrate demo database: `rake db:migrate`
- reset demo database: `rake db:drop db:create`
- run rails generators:
  - `rails generate administrate:field FooBar`
  - I was pleasantly surprised that this placed the files
    in the correct `spec/example_app` subdirectory
- access demo app console:: `rails console`
- view demo app routes: `rake routes`
- appraisal test suite: `bundle exec appraisal rake`

References:
-----------

http://stackoverflow.com/questions/3081699/deploy-a-subdirectory-to-heroku
c-lliope added a commit that referenced this pull request Nov 6, 2015
Problem:
--------

With the git subtree strategy adopted in #144,
there are several unconventional things
surrounding the heroku deploy process:

- Pushing to heroku requires a `git subtree` command
- We need special logic in the Gemfile in order to run the demo app
  locally with our local version of Administrate.
- `rake` and `rails` commands need to be run from the
  `spec/example_app` subdirectory.

In addition, there is a dependency problem between the example app
and the master branch of administrate on github.
The example app needs to pull the master branch
as a dependency on heroku,
but the `Gemfile.lock` locks it in to a specific version.

It is impossible to point the `Gemfile.lock`
at the repo's current commit,
because that would require a new commit
before we could deploy to Heroku.

The result would be that the branch on Heroku would always differ
from the branch on GitHub by at least one commit,
and we would need to develop special tooling around pushing to heroku.

Solution:
---------

Make the `rails`, `rake`, and `bundle exec` commands work
from the root directory of the repository.
This involved:

- Move the `config.ru` script to the root of the repo
- Update `rake` and `rails` binstubs to reference nested rails app
- Move `unicorn.rb` (required by Procfile) to top-level `config` dir
- Combine both Gemfiles into a single one, at the root of the repo
- Remove spring for simplicity
- Remove unused rake tasks

I tested the following tasks.
All work from the root directory of the repo:

- deploy to heroku: `git push heroku master`
- run server locally: `rails server` or `foreman start`
- seed demo database: `rake db:seed`
- migrate demo database: `rake db:migrate`
- reset demo database: `rake db:drop db:create`
- run rails generators:
  - `rails generate administrate:field FooBar`
  - I was pleasantly surprised that this placed the files
    in the correct `spec/example_app` subdirectory
- access demo app console:: `rails console`
- view demo app routes: `rake routes`
- appraisal test suite: `bundle exec appraisal rake`

References:
-----------

http://stackoverflow.com/questions/3081699/deploy-a-subdirectory-to-heroku
c-lliope added a commit that referenced this pull request Nov 12, 2015
- Update gem version
- Update README with recommended optimistic versioning for bundler
- Update README with warning about Administrate's pre-1.0 status
- Update CHANGELOG to fill in missing PR references
- sort CHANGELOG entries according to change type

Changes:

```
* [#191] [CHANGE] Improve API for specifying how resources are displayed
  across the dashboard.
  * Models are now displayed with a sensible default - (e.g. "User #2")
  * Users can define `ModelDashboard#display_resource(resource)` for custom
    display behavior
  * Users who have generated views for the following field types
    may need to update them to take advantage of the new API:
    * HasOne
    * HasMany
    * Polymorphic
    * BelongsTo
* [#223] [FEATURE] Translation: Vietnamese
* [#161] [FEATURE] Translation: Mandarin Chinese
* [#196] [FEATURE] Translation: Taiwanese Mandarin
* [#142] [FEATURE] Translation: Brazilian Portuguese
* [#171] [FEATURE] Translation: Polish
* [#153] [FEATURE] Translation: Russian
* [#148] [FEATURE] Translation: French
* [#147] [FEATURE] Translation: German
* [#154] [FEATURE] Translation: Spanish
* [#126] [UI] Preserve whitespace when rendering text fields
* [#194] [BUGFIX] Don't clear out datetime values in form fields
* [#193] [BUGFIX] Don't assume that unrecognized db column types are searchable
* [#124] [BUGFIX] Better detection of application models
* [#156] [COMPAT] Include missing `sass-rails` dependency in gemspec
* [#174] [COMPAT] Make several missing dependencies explicit.
* [#144] [COMPAT] Update repository structure so Bundler can pull the gem from github.
  (e.g. `gem "administrate", github: "thoughtbot/administrate"`)
* [#166] [COMPAT] Use ANSI SQL standards for case-insensitive search
* [#120] [DOC] Add Rubygems version badge to README
* [#165] [DOC] Add CircleCI badge to README
* [#119] [DOC] Add CodeClimate badge to README
```
c-lliope added a commit that referenced this pull request Nov 12, 2015
- Update gem version
- Update README with recommended optimistic versioning for bundler
- Update README with warning about Administrate's pre-1.0 status
- Update CHANGELOG to fill in missing PR references
- add `[I18n]` category to CHANGELOG
- sort CHANGELOG entries according to change type

Changes:

* [#191] [CHANGE] Improve API for specifying how resources are displayed
  across the dashboard.
  * Models are now displayed with a sensible default - (e.g. "User #2")
  * Users can define `ModelDashboard#display_resource(resource)` for custom
    display behavior
  * Users who have generated views for the following field types
    may need to update them to take advantage of the new API:
    * HasOne
    * HasMany
    * Polymorphic
    * BelongsTo
* [#223] [FEATURE] Translation: Vietnamese
* [#161] [FEATURE] Translation: Mandarin Chinese
* [#196] [FEATURE] Translation: Taiwanese Mandarin
* [#142] [FEATURE] Translation: Brazilian Portuguese
* [#171] [FEATURE] Translation: Polish
* [#153] [FEATURE] Translation: Russian
* [#148] [FEATURE] Translation: French
* [#147] [FEATURE] Translation: German
* [#154] [FEATURE] Translation: Spanish
* [#126] [UI] Preserve whitespace when rendering text fields
* [#194] [BUGFIX] Don't clear out datetime values in form fields
* [#193] [BUGFIX] Don't assume that unrecognized db column types are searchable
* [#124] [BUGFIX] Better detection of application models
* [#156] [COMPAT] Include missing `sass-rails` dependency in gemspec
* [#174] [COMPAT] Make several missing dependencies explicit.
* [#144] [COMPAT] Update repository structure so Bundler can pull the gem from github.
  (e.g. `gem "administrate", github: "thoughtbot/administrate"`)
* [#166] [COMPAT] Use ANSI SQL standards for case-insensitive search
* [#120] [DOC] Add Rubygems version badge to README
* [#165] [DOC] Add CircleCI badge to README
* [#119] [DOC] Add CodeClimate badge to README
@c-lliope c-lliope mentioned this pull request Nov 13, 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.

None yet

5 participants