Skip to content

Commit

Permalink
Install Prettier to format everything it's able to format
Browse files Browse the repository at this point in the history
  • Loading branch information
erbridge authored and tahb committed Oct 30, 2020
1 parent fc1546d commit 5aa723a
Show file tree
Hide file tree
Showing 21 changed files with 403 additions and 227 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ jobs:
- name: Set up Docker
run: docker network create test
- name: Set up Postgres
run: docker run -d --name pg --network test -e POSTGRES_USER=test -e POSTGRES_HOST_AUTH_METHOD=trust -p 5432:5432 postgres:11
run:
docker run -d --name pg --network test -e POSTGRES_USER=test -e
POSTGRES_HOST_AUTH_METHOD=trust -p 5432:5432 postgres:11
- name: Build a new Docker image
run: docker build . --build-arg RAILS_ENV=test -t app:test
- name: Run the tests
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,5 @@ config/master.key
.env.*
!.env.example
!.env.test

/node_modules/
25 changes: 25 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
!/*/
!/.prettierrc
!/*.json
!/*.md
/.bundle/
/bin/
/log/
/script/
/storage/
/tmp/
/vendor/
.keep
*.coffee
*.enc
*.erb
*.haml
*.ico
*.jpg
*.png
*.rake
*.rb
*.ru
*.sh
*.txt
3 changes: 3 additions & 0 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"proseWrap": "always"
}
31 changes: 22 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
# Using this template

1. Search for `TODO` across the repository to customise the template to the new
project
project
1. Find and replace `rails-template` across the repository
1. Be aware of [dxw RFCs](https://github.com/dxw/tech-team-rfcs), especially
those that have not resulted in a default code change in this repository:
- [rfc-013-use-docker-to-deploy-and-run-applications-in-containers](https://github.com/dxw/tech-team-rfcs/blob/master/rfc-013-use-docker-to-deploy-and-run-applications-in-containers.md)
those that have not resulted in a default code change in this repository:
- [rfc-013-use-docker-to-deploy-and-run-applications-in-containers](https://github.com/dxw/tech-team-rfcs/blob/master/rfc-013-use-docker-to-deploy-and-run-applications-in-containers.md)

TODO: Remove this section from the README once complete

Expand All @@ -20,7 +21,9 @@ TODO: Add a summary of who the application is for and what it will do.

1. copy `/.env.example` into `/.env.development.local`.

Our intention is that the example should include enough to get the application started quickly. If this is not the case, please ask another developer for a copy of their `/.env.development.local` file.
Our intention is that the example should include enough to get the
application started quickly. If this is not the case, please ask another
developer for a copy of their `/.env.development.local` file.

TODO: Add getting started steps

Expand All @@ -30,12 +33,15 @@ TODO: Add testing instructions

## Running Brakeman

Run [Brakeman](https://brakemanscanner.org/) to highlight any security vulnerabilities:
Run [Brakeman](https://brakemanscanner.org/) to highlight any security
vulnerabilities:

```bash
brakeman
```

To pipe the results to a file:

```bash
brakeman -o report.text
```
Expand All @@ -59,14 +65,21 @@ in doc/architecture/decisions and contributed to with the

## Managing environment variables

We use [Dotenv](https://github.com/bkeepers/dotenv) to manage our environment variables locally.
We use [Dotenv](https://github.com/bkeepers/dotenv) to manage our environment
variables locally.

The repository will include safe defaults for development in `/.env.example` and for test in `/.env.test`. We use 'example' instead of 'development' (from the Dotenv docs) to be consistent with current dxw conventions and to make it more explicit that these values are not to be committed.
The repository will include safe defaults for development in `/.env.example` and
for test in `/.env.test`. We use 'example' instead of 'development' (from the
Dotenv docs) to be consistent with current dxw conventions and to make it more
explicit that these values are not to be committed.

To manage sensitive environment variables:

1. Add the new key and safe default value to the `/.env.example` file eg. `ROLLBAR_TOKEN=ROLLBAR_TOKEN`
2. Add the new key and real value to your local `/.env.development.local` file, which should never be checked into Git. This file will look something like `ROLLBAR_TOKEN=123456789`
1. Add the new key and safe default value to the `/.env.example` file eg.
`ROLLBAR_TOKEN=ROLLBAR_TOKEN`
1. Add the new key and real value to your local `/.env.development.local` file,
which should never be checked into Git. This file will look something like
`ROLLBAR_TOKEN=123456789`

## Access

Expand Down
5 changes: 2 additions & 3 deletions app/assets/javascripts/cable.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
//= require_self
//= require_tree ./channels

(function() {
(function () {
this.App || (this.App = {});

App.cable = ActionCable.createConsumer();

}).call(this);
}.call(this));
2 changes: 1 addition & 1 deletion app/assets/stylesheets/1st_load_framework.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ img {
// override for the 'Home' navigation link
.navbar-brand {
font-size: inherit;
}
}

// THESE ARE EXAMPLES YOU CAN MODIFY
// create your own classes
Expand Down
1 change: 0 additions & 1 deletion config/storage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ test:
local:
service: Disk
root: <%= Rails.root.join("storage") %>

# Use rails credentials:edit to set the AWS secrets (as aws:access_key_id|secret_access_key)
# amazon:
# service: S3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ We need to record the architectural decisions made on this project.

## Decision

We will use Architecture Decision Records, as [described by Michael Nygard](http://thinkrelevance.com/blog/2011/11/15/documenting-architecture-decisions).
We will use Architecture Decision Records, as
[described by Michael Nygard](http://thinkrelevance.com/blog/2011/11/15/documenting-architecture-decisions).

## Consequences

See Michael Nygard's article, linked above. For a lightweight ADR toolset, see Nat Pryce's [adr-tools](https://github.com/npryce/adr-tools).
See Michael Nygard's article, linked above. For a lightweight ADR toolset, see
Nat Pryce's [adr-tools](https://github.com/npryce/adr-tools).
40 changes: 31 additions & 9 deletions doc/architecture/decisions/0002-use-pull-request-templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,41 @@ Accepted

## Context

The quality of information included in our pull requests varies greatly which can lead to code reviews which take longer and are harder for the person to understand the considerations, outcomes and consquences of a series of changes.
The quality of information included in our pull requests varies greatly which
can lead to code reviews which take longer and are harder for the person to
understand the considerations, outcomes and consquences of a series of changes.

A couple of recent projects have found a GitHub pull request template to have been a positive change. Prompting what pull request descriptions should include has lead to better documented changes that have been easier to review on the whole.
A couple of recent projects have found a GitHub pull request template to have
been a positive change. Prompting what pull request descriptions should include
has lead to better documented changes that have been easier to review on the
whole.

## Decision

Include a basic pull request template for GitHub so that every pull request prompts every author to fill it out.
Include a basic pull request template for GitHub so that every pull request
prompts every author to fill it out.

## Consequences

- a small overhead to every pull request which may prompt us to spend more time in creating a pull request. We accept the cost of this to gain the benefits of easier reviews, and better documented changes
- writing good pull request descriptions is not to be done instead of commit messages, they are the primary source of storing rationale that will persist. Commit messages may be duplicated into the pull request description.
- prompting authors to articulate their thought process and decision making can improve the quality of code before a reviewer becomes involved. Essentially you are following the process of [rubber ducking](https://en.wikipedia.org/wiki/Rubber_duck_debugging) which gives you the final opportunity to amend your proposal
- this is not compatible when source code is hosted on other services eg. [GitLab requires a file of a different name](https://docs.gitlab.com/ee/user/project/description_templates.html#creating-merge-request-templates), as all of our Rails projects are on GitHub this should be a minor issue
- asking for screenshots doesn't always make sense especially when production deployments are facilitated by pull requests from develop into master. We accept that in these cases the cost of editing the template is small enough to not be a problem. The first header that prompts to describe changes does still apply
- not all pull requests result in a frontend change that requires screenshots, we believe that these prompts can be removed quickly with a minor cost to the author
- a small overhead to every pull request which may prompt us to spend more time
in creating a pull request. We accept the cost of this to gain the benefits of
easier reviews, and better documented changes
- writing good pull request descriptions is not to be done instead of commit
messages, they are the primary source of storing rationale that will persist.
Commit messages may be duplicated into the pull request description.
- prompting authors to articulate their thought process and decision making can
improve the quality of code before a reviewer becomes involved. Essentially
you are following the process of
[rubber ducking](https://en.wikipedia.org/wiki/Rubber_duck_debugging) which
gives you the final opportunity to amend your proposal
- this is not compatible when source code is hosted on other services eg.
[GitLab requires a file of a different name](https://docs.gitlab.com/ee/user/project/description_templates.html#creating-merge-request-templates),
as all of our Rails projects are on GitHub this should be a minor issue
- asking for screenshots doesn't always make sense especially when production
deployments are facilitated by pull requests from develop into master. We
accept that in these cases the cost of editing the template is small enough to
not be a problem. The first header that prompts to describe changes does still
apply
- not all pull requests result in a frontend change that requires screenshots,
we believe that these prompts can be removed quickly with a minor cost to the
author
11 changes: 8 additions & 3 deletions doc/architecture/decisions/0003-use-standard-rb.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@ Accepted

## Context

We need to make sure our code is written in a standard style for clarity, consistency across a project and to avoid back and forth between developers about code style.
We need to make sure our code is written in a standard style for clarity,
consistency across a project and to avoid back and forth between developers
about code style.

## Decision

We will use [Standard.rb](https://github.com/testdouble/standard) and run the standard.rb rake task to lint the code as part of the test suite.
We will use [Standard.rb](https://github.com/testdouble/standard) and run the
standard.rb rake task to lint the code as part of the test suite.

## Consequences

Code must be written to match the standard.rb style, otherwise the test suite will fail. Most errors can be automatically fixed by running `rake standard:fix`.
Code must be written to match the standard.rb style, otherwise the test suite
will fail. Most errors can be automatically fixed by running
`rake standard:fix`.
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,44 @@ Accepted

Accessing ENV directly without a wrapper is limited and can introduce problems.

We want our tooling to help us guard against missing environment variables. When `nil` is accidentally provided during the start up process it is preferable to fail fast with an explicit message. Without this `nil` can be passed down through the stack and cause strange behaviour where the code has been designed with it as a dependency. Instead of adding `nil` guards throughout the codebase for required environment variables (eg. `ENV.fetch('FOO', 'default')`, this should be managed centrally.

We have previously used Figaro for this purpose but it was deprecated in 2016 https://github.com/laserlemon/figaro. We should only use supported gems to ensure we get support in the form of fixes and security patches.

We also want to be able to stub our environment variables in our test suite. An easy example of this is when we use environment variables as a feature flag mechanism. We want to stub the value to test both scenarios without being influenced by real values being loaded. Mutating the actual ENV value (eg. ` allow(ENV).to receive(:[]).with('BOX_ID').and_return("1234")`) is possible but may have unexpected consequences where more than 1 part of the same process under test uses the same variable. Figaro used to be a handy abstraction layer that we could stub eg. `allow(Figaro).to receive(:env).with(:foo).and_return('bar')`. We should then consider how we can stub environment variables.
We want our tooling to help us guard against missing environment variables. When
`nil` is accidentally provided during the start up process it is preferable to
fail fast with an explicit message. Without this `nil` can be passed down
through the stack and cause strange behaviour where the code has been designed
with it as a dependency. Instead of adding `nil` guards throughout the codebase
for required environment variables (eg. `ENV.fetch('FOO', 'default')`, this
should be managed centrally.

We have previously used Figaro for this purpose but it was deprecated in 2016
https://github.com/laserlemon/figaro. We should only use supported gems to
ensure we get support in the form of fixes and security patches.

We also want to be able to stub our environment variables in our test suite. An
easy example of this is when we use environment variables as a feature flag
mechanism. We want to stub the value to test both scenarios without being
influenced by real values being loaded. Mutating the actual ENV value (eg.
`allow(ENV).to receive(:[]).with('BOX_ID').and_return("1234")`) is possible but
may have unexpected consequences where more than 1 part of the same process
under test uses the same variable. Figaro used to be a handy abstraction layer
that we could stub eg.
`allow(Figaro).to receive(:env).with(:foo).and_return('bar')`. We should then
consider how we can stub environment variables.

## Decision

Use DotEnv to load our environment variables.

## Consequences

Should Docker and Docker Compose be added to the project the environment variables will need to be loaded with `docker-compose up --env-file=.env.development` rather than `docker-compose.env` which is a pattern we have used. Having 2 files for managing environment variables such as `.env*` and `docker-compose.env*` is undesirable due to the overhead in keeping these in sync.

DotEnv loads environment variables but doesn't offer an interface as Figaro did. For DotEnv you'd access by writing `ENV['foo']` rather than `DotEnv.foo`. We will need to make a supporting decision to use [Climate Control](https://thoughtbot.com/blog/testing-and-environment-variables#use-climate-control) to support testing.
Should Docker and Docker Compose be added to the project the environment
variables will need to be loaded with
`docker-compose up --env-file=.env.development` rather than `docker-compose.env`
which is a pattern we have used. Having 2 files for managing environment
variables such as `.env*` and `docker-compose.env*` is undesirable due to the
overhead in keeping these in sync.

DotEnv loads environment variables but doesn't offer an interface as Figaro did.
For DotEnv you'd access by writing `ENV['foo']` rather than `DotEnv.foo`. We
will need to make a supporting decision to use
[Climate Control](https://thoughtbot.com/blog/testing-and-environment-variables#use-climate-control)
to support testing.
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,19 @@ Accepted

## Context

It can be easy to miss an inefficient database query during code review. These can build up and have detremental performance on the application and effect the user experience.
It can be easy to miss an inefficient database query during code review. These
can build up and have detremental performance on the application and effect the
user experience.

## Decision

Add an automatic check to the test suite to ensure (through CI) that these are fixed before being deployed.
Add an automatic check to the test suite to ensure (through CI) that these are
fixed before being deployed.

## Consequences

- Code reviews are depended upon less to catch this type of error, removing the risk of human error
- Code reviews are depended upon less to catch this type of error, removing the
risk of human error
- Application response times should no longer be affected by this type of issue
- Requires protected branch configuration to ensure that CI must succeed before being able to be merged
- Requires protected branch configuration to ensure that CI must succeed before
being able to be merged
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,23 @@ Accepted

## Context

We need a mechanism to capture and manage application errors. Without an integration our debugging options are to access a console on live environments and try to replicate (something we want to minimise) or by looking through information provided by logs.
We need a mechanism to capture and manage application errors. Without an
integration our debugging options are to access a console on live environments
and try to replicate (something we want to minimise) or by looking through
information provided by logs.

We have used Rollbar for a few years now and we have not reviewed this decision since. It is currently being used for 14 applications.
We have used Rollbar for a few years now and we have not reviewed this decision
since. It is currently being used for 14 applications.

For some projects we use their technical tooling of choice to aid in the transition to business as usual. Due to this we will have a handful of projects using Sentry and possible others.
For some projects we use their technical tooling of choice to aid in the
transition to business as usual. Due to this we will have a handful of projects
using Sentry and possible others.

Sometimes Rollbar environment names don't match the Rails environment. Dalmatian-<project> and paas-<project> both exist. There also exists both permutations for the same project as we transition. We have used ROLLBAR_ENV to manage this before so making it explicit will hopefully make it clearer how it can be changed.
Sometimes Rollbar environment names don't match the Rails environment.
Dalmatian-<project> and paas-<project> both exist. There also exists both
permutations for the same project as we transition. We have used ROLLBAR_ENV to
manage this before so making it explicit will hopefully make it clearer how it
can be changed.

## Decision

Expand All @@ -24,5 +34,9 @@ Use Rollbar to collect and manage our application errors.

- the Rails team will have a single tool to be familiar with
- there is only one tool new joiners will need to be invited to
- administering and paying for a single service is easier for dxw however there is a risk that sharing the same account with many projects may go over the usage threshold
- in the rarer case where we have to use the client tooling the team would have to accept an overhead of replace this configuration with one compatible with the tool of choice
- administering and paying for a single service is easier for dxw however there
is a risk that sharing the same account with many projects may go over the
usage threshold
- in the rarer case where we have to use the client tooling the team would have
to accept an overhead of replace this configuration with one compatible with
the tool of choice
Loading

0 comments on commit 5aa723a

Please sign in to comment.