Skip to content

Suggested maintenance of readme and template#13

Open
oma-s wants to merge 4 commits into
mainfrom
suggested_maintenance_of_readme_and_template
Open

Suggested maintenance of readme and template#13
oma-s wants to merge 4 commits into
mainfrom
suggested_maintenance_of_readme_and_template

Conversation

@oma-s

@oma-s oma-s commented Nov 19, 2023

Copy link
Copy Markdown

This PR is meant for keeping the repo up to date. I updated the template and did some changes to the readme to reflect the current way we are handling the dev environment.

Comment thread readme.md

bundle install

Or for short just

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need to include the short hand in our docs. 🤔

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I can remove it. I felt it does not hurt to go a bit into detail though.

Comment thread readme.md
@@ -89,12 +97,6 @@ If `bundle install` fails for the GEM `pg`, install `postgresql`:

brew install postgresql

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this will not work on the projects that need postgres 15.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

that line has not changed. What change would you suggest?

Comment thread readme.md
### Auto-Start Mode

To have the environment start automatically when _Docker_ start, execute the following command once.
To have the environment start automatically when _Docker_ starts, execute the following command once.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we don't use docker-compose on most projects at the moment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

that is why I removed it in lines 92-97. As of Docker Compose V2 it uses the syntax below in line 11

@danieldiekmeier danieldiekmeier Nov 28, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We don't use Docker Compose directly in the projects, but (at least for me) the development environment (Postgres, MySQL, Redis, Elasticsearch, etc) is running via Docker Compose the whole time. I assumed that's what these lines are referencing.

Comment thread rails-template.rb
gem "pry-rails"
gem "guard"
gem "guard-rspec"
gem "solargraph-standardrb"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we also dropped solar graph.

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

postgres post should also be 54313

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line should also be removed config.assets.paths << Rails.root.join("node_modules")

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't use solar graph anymore...

  plugins:
    - solargraph-standardrb
  reporters:
    - standardrb
YML```

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

file ".env", <<~ENV
REDIS_URL=redis://127.0.0.1:6379

ease development with this puma settings

WEB_CONCURRENCY=0
PUMA_WORKER_TIMEOUT=6000
RAILS_MAX_THREADS=20
ENV

we don't user .env files for projects.

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

file "package.json", <<~JS
{
"scripts": {
"format": "prettier --write \"src//*.{tsx,ts,scss,json}\"",
"lint": "yarn lint:types && yarn lint:style && yarn lint:format",
"lint:types": "tsc --noEmit",
"lint:style": "eslint app/javascript/
/.ts --max-warnings 0",
"lint:format": "prettier --list-different \"app/**/
.{ts,scss,json}\"",
"build": "esbuild app/javascript/. --bundle --sourcemap --outdir=app/assets/builds",
"bundle-size": "npx source-map-explorer app/assets/builds/application.js app/assets/builds/application.js.map --no-border-checks",
"start": "yarn build --watch",
"live": "yarn livereload -e scss app/assets"
},
"license": "MIT"
}
JS

remove livereload as it doesnt' work.

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All the javascript files should use double quotes

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

file ".github/workflows/ci.yml", <<~YML
name: CI
on: [push]
jobs:
ruby:
runs-on: ubuntu-20.04

  Update this to 22.04

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

file "spec/support/system/cuprite_setup.rb", <<~RUBY

frozen_string_literal: true

require "capybara/cuprite"

Then, we need to register our driver to be able to use it later

with #driven_by method.

Capybara.register_driver(:cuprite) do |app|
Capybara::Cuprite::Driver.new(
app,
**{
window_size: [1200, 1400],
# See additional options for Dockerized environment in the respective section of this article
browser_options: {},
# Increase Chrome startup wait time (required for stable CI builds)
process_timeout: 10,
# Enable debugging capabilities
inspector: true,
# Allow running Chrome in a headful mode by setting HEADLESS env
# var to a falsey value
headless: Config.headless?
}
)
end

Config.headless?(default: true)

Comment thread rails-template.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

run "yarn add -D @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier eslint-plugin-prettier prettier livereload"

remove live reload

@danieldiekmeier danieldiekmeier left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you! I think this is a good small set of changes to get the ball rolling. I personally would rather see this merged now than trying to fix every little thing at once.

Comment thread readme.md Outdated

## node

Make sure you have node installed. In order to do that use a version manager of your choice, such as [nvm](https://github.com/nvm-sh/nvm) or [volta](https://volta.sh/).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no choice, use volta ;-)

@oma-s oma-s self-assigned this Nov 30, 2023
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.

4 participants