Pull request to simplify deployment and improve relative URL handling


(Vadim Geshel) #1

Hello, I’ve submitted a pull request a little while ago and was wondering if I could get some feedback on it? Thanks!

PR description, for convenience:

This set of changes does two things. They are mostly unrelated but they were essential for us in building our Discourse instance and, I believe, they are generally useful.

  1. Better support for RAILS_RELATIVE_URL: we have fixed numerous places where it wasn’t respected.
    Running under a a relative URL (domain.com/forum/…) vs subdomain (forum.domain.com) is very useful from the SEO point of view, see, for instance, here.

  2. Better support for deployments with external Postgres, Redis and nginx.
    The repo contains a Dockerfile which builds an image that includes only Discourse itself. This makes it easier to deploy, e.g., in AWS, using RDS (hosted Postgres) and ElastiCache (hosted Redis). The repo also contains a fig.yml, which helps you use fig to do local development using an equivalent setup (the Vagrant-based setup still works just fine). The fig setup is documented in docs/fig.md.

(Gerhard Schlager) #2

I’m looking forward to trying out fig.
I hope we will see this getting merged after 1.2 is released.

(Jens Maier) #3

First of all, you should clean up your commit history. Every commit should be self-contained, i.e. must pass tests, and commits must have meaningful titles; titles like “fixes” and “let’s try this” are straight examples for VCS anti-patterns. Avoid whitespace-only changes (e.g. in README.md).

When you have a set of clean, well-described commits, rebase onto tests-passed and update the PR.

Also, support for Discourse in a subdirectory was apparently removed, so you’re basically trying to re-introduce a feature the devs have abandoned. Since this is a significant change to Discourse’s design, I don’t see this feature getting accepted without better test coverage.

(Vadim Geshel) #4


Will do re: commit history. CONTRIBUTING says to rebase to master, are you sure about the tests-passed bit?

I’m aware of the history of subdirectory support. Yes, I am hoping to reintroduce the subject because I believe it’s essential (it’s certainly essential for us). It is not a significant change as demonstrated by this PR. It takes only a few fairly small/simple changes to make it work.

(Jens Maier) #5

Under normal circumstances, tests-passed trails behind master by less then 10 minutes; most of the time it is identical. The advantage of rebasing to tests-passed is that you can be sure that if tests do not pass, the fault is somewhere in your code and not in something that was recently introduced in master.

(Vadim Geshel) #6

Thank you. I’ve had to push a new branch with a cleaned up commit history and create a new pull request and close the old one.