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

Quality of life improvements #1292

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
95a6e93
Changed discord as the primary way to get in touch with the comminuty
crimson-knight May 17, 2022
7462cc1
Changed the discord link so that it uses a permanent invite link
crimson-knight May 17, 2022
72d9e6b
Fixed the version number being out dated from the actual latest relea…
crimson-knight May 17, 2022
72bdd15
Merge branch 'amberframework:master' into master
crimson-knight May 17, 2022
da256f3
Merge branch 'amberframework:master' into master
crimson-knight May 24, 2022
16a8288
Merge branch 'amberframework:master' into master
crimson-knight May 25, 2022
5cf7631
Make default package support older and newer versions of node-sass
crimson-knight Aug 23, 2022
aa1941e
Bump amber version and update Ameba version
crimson-knight Aug 23, 2022
733152a
Added github actions for CI instead of TravisCI
crimson-knight Aug 23, 2022
84acb1a
Amber version bump for these quality of life updates
crimson-knight Aug 23, 2022
5ddc56e
Fix naming warning raised by crystal compiler about positional names …
crimson-knight Aug 23, 2022
1cb910d
removed the travisCI yml
crimson-knight Aug 23, 2022
5287e42
Fixed amber_spec cookie naming error that relied on old implicit enco…
crimson-knight Aug 23, 2022
167b620
Fixed areas that ameba caught for format improvements
crimson-knight Aug 23, 2022
05139bb
Changed ubuntu to an older version instead of latest
crimson-knight Aug 23, 2022
1d52c9d
Changed ci workflow to remove an extra sqlite3 dev library that may b…
crimson-knight Aug 23, 2022
34e8536
Changed from uninstall to install to fix sqlite3 issue
crimson-knight Aug 23, 2022
02674c7
Changed installation of sqlite3 based on further reading
crimson-knight Aug 23, 2022
0cd0a53
Set apt install to automatically agree to prompts
crimson-knight Aug 23, 2022
9cac540
changed packed installed from full sqlite3 to sqlite3-dev
crimson-knight Aug 23, 2022
db6cade
Adds redis for the test suite
crimson-knight Aug 23, 2022
78954a6
Added redis dependencies to the apt install step
crimson-knight Aug 23, 2022
3e49484
Changed steps to stop using the supercharge redis installation and ad…
crimson-knight Aug 24, 2022
84e1162
Trying redis as a service
crimson-knight Aug 24, 2022
ab39ee1
Tweaked the redis service settings to map the port for the host conta…
crimson-knight Aug 24, 2022
8c82e46
Added a step to check access to the redis service before the tests run
crimson-knight Aug 24, 2022
29ece19
redis-cli stopped existing so trying to install plain redis
crimson-knight Aug 24, 2022
46c25f8
removed the port mapping to test if redis becomes accessible from the…
crimson-knight Aug 24, 2022
8345859
specified the host when testing the redis connection
crimson-knight Aug 24, 2022
78b3bc3
Added ports mapping back to the redis service container to see if thi…
crimson-knight Aug 24, 2022
9094d9b
Changed host from localhost to redis to see if that maps correctly
crimson-knight Aug 24, 2022
7365ed4
Added the REDIS_URL with the hostname of the redis service and remove…
crimson-knight Aug 24, 2022
875785d
Changed the empty? method to be more clear about 'any origin' being t…
crimson-knight Aug 24, 2022
2325ccb
Include the discord link into the generated home page templates
crimson-knight Sep 14, 2022
10f1ced
Tweaked how the PG test data is configured
crimson-knight Sep 14, 2022
c44e3b8
Merge branch 'master' into feature/qol-improvements
crimson-knight Sep 14, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .ameba.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Excluded:
- tmp/
- myapp/
46 changes: 46 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
on:
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to be running all the different combinations of tests. I thought we ran against postgres, mysql and sqlite. Also, I thought we tested different templating options using docker compose.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll look into expanding the combination of tests more. I'm not very familiar with GHA and I'm just trying to make it work to start.

Copy link
Member Author

Choose a reason for hiding this comment

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

And FYI I still have 10 failing tests to fix, so I won't merge this until at least everything is green on my repo

push:
jobs:
amber-spec:

runs-on: ubuntu-latest

container:
image: crystallang/crystal

services:
redis:
image: redis
options: >-
--health-cmd "redis-cli ping"
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 6379:6379
postgres:
image: postgres
env:
POSTGRES_PASSWORD: postgres
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 5432:5432

steps:
- name: Update & install sqlite3
run: apt update && apt install -y libsqlite3-dev redis

- name: Download source
uses: actions/checkout@v3

- name: Install shards
run: shards install

- name: Run tests
run: bin/amber_spec
env:
REDIS_URL: "redis://redis:6379"
6 changes: 3 additions & 3 deletions shard.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: amber

version: 1.2.1
version: 1.3.0

authors:
- Amber Team and Contributors <amberframework.org>
Expand Down Expand Up @@ -52,7 +52,7 @@ dependencies:

redis:
github: stefanwille/crystal-redis
version: ~> 2.7.0
version: ~> 2.8.0

shell-table:
github: luckyframework/shell-table.cr
Expand Down Expand Up @@ -85,4 +85,4 @@ dependencies:
development_dependencies:
ameba:
github: crystal-ameba/ameba
version: ~> 0.13.4
version: ~> 1.0.0
2 changes: 1 addition & 1 deletion spec/amber/cli/commands/pipelines/pipelines_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ module Amber::CLI
end

pipe_plugs.each do |pipe_name, plugs|
output_plugs = output_lines.select { |line| line.includes?(pipe_name) }
output_plugs = output_lines.select(&.includes?(pipe_name))
plugs.each_with_index do |plug, index|
output_plug = output_plugs[index]
(output_plug != nil).should be_true
Expand Down
6 changes: 3 additions & 3 deletions spec/amber/cli/commands/routes/routes_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ module Amber::CLI

expected = "Amber::Controller::Static"
output.should contain expected
line = output_lines.find("") { |this_line| this_line.includes? expected }
line = output_lines.find("", &.includes?(expected))
expectations = %w(get Amber::Controller::Static index static /*)
expectations.each do |expectation|
line.should contain expectation
end

expected = "HomeController"
output.should contain expected
line = output_lines.find("") { |this_line| this_line.includes? expected }
line = output_lines.find("", &.includes?(expected))
expectations = %w(get HomeController index web /)
expectations.each do |expectation|
line.should contain expectation
Expand Down Expand Up @@ -77,7 +77,7 @@ module Amber::CLI

expected = "websocket"
output.should contain expected
line = output_lines.find("") { |this_line| this_line.includes? expected }
line = output_lines.find("", &.includes?(expected))
expectations = %w(websocket ElectricSocket web /electric)
expectations.each do |expectation|
line.should contain expectation
Expand Down
2 changes: 1 addition & 1 deletion spec/amber/router/session/redis_store_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ module Amber::Router::Session
cookie_store.delete("ses")

cookie_store.to_h.keys.should eq %w(a b c)
cookie_store.to_h["c"].should eq "x"
cookie_store.to_h.dig("c").should eq "x"
end
end

Expand Down
2 changes: 1 addition & 1 deletion src/amber/cli/commands/exec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module Amber::CLI
_filename = if File.exists?(args.code)
args.code
elsif options.back.to_i(strict: false) > 0
Dir.glob("./tmp/*_console.cr").sort.reverse[options.back.to_i(strict: false) - 1]?
Dir.glob("./tmp/*_console.cr").sort.reverse![options.back.to_i(strict: false) - 1]?
end

system("cp #{_filename} #{@filename}") if _filename
Expand Down
4 changes: 2 additions & 2 deletions src/amber/cli/commands/pipelines.cr
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ module Amber::CLI
match = line.match(PIPELINE_REGEX)

if match && (pipes = match[1])
pipes = pipes.split(/,\s*/).map { |s| s.gsub(/[:\"]/, "") }
pipes = pipes.split(/,\s*/).map(&.gsub(/[:\"]/, ""))
result << {pipes: pipes, plugs: [] of String}
else
raise BadRoutesException.new(FAILED_TO_PARSE_ERROR)
Expand All @@ -113,7 +113,7 @@ module Amber::CLI
table.border_color = :dark_gray unless options.no_color?

if options.no_plugs?
result.map { |pipes_and_plugs| pipes_and_plugs[:pipes] }.flatten.uniq.each do |pipe|
result.flat_map { |pipes_and_plugs| pipes_and_plugs[:pipes] }.uniq!.each do |pipe|
row = table.add_row
row.add_column(pipe)
end
Expand Down
2 changes: 1 addition & 1 deletion src/amber/cli/commands/plugin.cr
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module Amber::CLI

if Amber::Plugins::Plugin.can_generate?(args.name)
template = Amber::Plugins::Plugin.new(args.name, "./src/plugins", options.args)
template.generate (options.uninstall? ? "uninstall" : "install")
template.generate(options.uninstall? ? "uninstall" : "install")
end
end

Expand Down
2 changes: 1 addition & 1 deletion src/amber/cli/commands/routes.cr
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ module Amber::CLI

private def print_routes_table_json
puts routes.map { |route|
route.transform_keys { |key| key.to_s.downcase.gsub(' ', '_') }
route.transform_keys(&.to_s.downcase.gsub(' ', '_'))
}.to_json
end

Expand Down
2 changes: 1 addition & 1 deletion src/amber/cli/helpers/process_runner.cr
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ module Sentry
private def check_processes
@processes.each do |task, procs|
# clean up process list and restart if terminated
if procs.any?
if !procs.empty?
procs.reject!(&.terminated?)

if procs.empty?
Expand Down
2 changes: 1 addition & 1 deletion src/amber/cli/recipes/scaffold/controller.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module Amber::Recipes::Scaffold
@fields += %w(created_at:time updated_at:time).map do |f|
Amber::CLI::Field.new(f, hidden: true, database: @database)
end
@visible_fields = @fields.reject { |f| f.hidden }
@visible_fields = @fields.reject(&.hidden)
field_hash

@template = RecipeFetcher.new("scaffold", @recipe).fetch
Expand Down
2 changes: 1 addition & 1 deletion src/amber/cli/recipes/scaffold/view.cr
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module Amber::Recipes::Scaffold
Amber::CLI::Field.new(f, hidden: true, database: @database)
end

@visible_fields = @fields.reject { |f| f.hidden }
@visible_fields = @fields.reject(&.hidden)

@template = RecipeFetcher.new("scaffold", @recipe).fetch
unless @template.nil?
Expand Down
6 changes: 0 additions & 6 deletions src/amber/cli/templates/app/.travis.yml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ redis_url: "redis://localhost:6379"
when "mysql" -%>
database_url: mysql://root@localhost:3306/<%= database_name %>_test
<% when "pg" -%>
database_url: postgres://postgres:password@localhost:5432/<%= database_name %>_test
database_url: <%= "postgres://postgres:#{ENV["POSTGRES_PASSWORD"]? || "password"}@localhost:5432/#{database_name}_test" %>
<% when "sqlite" -%>
database_url: sqlite3:./db/<%= database_name %>_test.db
<% else
Expand Down
5 changes: 5 additions & 0 deletions src/amber/cli/templates/app/package.json.ecr
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@
"file-loader": "^6.2.0",
"html-webpack-plugin": "^5.3.1",
"mini-css-extract-plugin": "^1.6.0",
<% if `node -v`.matches?(/v14|v16/) -%>
"node-sass": "^5.0.0",
"sass-loader": "^11.0.1",
<% else -%>
"node-sass": "^7.0.0",
"sass-loader": "^13.0.2",
<% end -%>
"webpack": "^5.36.2",
"webpack-cli": "^4.6.0",
"webpack-merge": "^5.7.3"
Expand Down
3 changes: 2 additions & 1 deletion src/amber/cli/templates/app/src/views/home/index.ecr.ecr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
<p>Thank you for trying out the Amber Framework. We are working hard to provide a super fast and reliable framework that provides all the productivity tools you are used to without sacrificing the speed.</p>
<div class="list-group">
<a class="list-group-item list-group-item-action" target="_blank" href="https://docs.amberframework.org">Getting Started with Amber Framework</a>
<a class="list-group-item list-group-item-action" target="_blank" href="https://github.com/veelenga/awesome-crystal">List of Awesome Crystal projects and shards</a>
<a class="list-group-item list-group-item-action" target="_blank" href="https://github.com/veelenga/awesome-crystal">List of Awesome C
<a class="list-group-item list-group-item-action" target="_blank" href="https://discord.gg/vwvP5zakSn">Join the Amber Discord!</a>
</div>
</div>
</div>
1 change: 1 addition & 0 deletions src/amber/cli/templates/app/src/views/home/index.slang.ecr
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
.list-group
a.list-group-item.list-group-item-action target="_blank" href="https://docs.amberframework.org" Getting Started with Amber Framework
a.list-group-item.list-group-item-action target="_blank" href="https://github.com/veelenga/awesome-crystal" List of Awesome Crystal projects and shards
a.list-grouo-item.list-group-item-action target="_blank" href="https://discord.gg/vwvP5zakSn" Join the Amber Discord!
2 changes: 1 addition & 1 deletion src/amber/controller/helpers/responders.cr
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ module Amber::Controller::Helpers
accept = context.request.headers["Accept"]?
if accept && !accept.empty?
accepts = accept.split(";").first?.try(&.split(Content::ACCEPT_SEPARATOR_REGEX))
return accepts if !accepts.nil? && accepts.any?
return accepts if !accepts.nil? && !accepts.empty?
end
end

Expand Down
6 changes: 3 additions & 3 deletions src/amber/pipes/cors.cr
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ module Amber
private def put_response_headers(response)
response.headers[Headers::ALLOW_CREDENTIALS] = @credentials.to_s if @credentials
response.headers[Headers::ALLOW_ORIGIN] = @origin.request_origin.not_nil!
response.headers[Headers::VARY] = vary unless @origin.any?
response.headers[Headers::VARY] = vary unless @origin.any_origin?
end

private def vary
Expand Down Expand Up @@ -122,7 +122,7 @@ module Amber
def match?(request)
return false if @origins.empty?
return false unless origin_header?(request)
return true if any?
return true if any_origin?

@origins.any? do |origin|
case origin
Expand All @@ -132,7 +132,7 @@ module Amber
end
end

def any?
def any_origin?
@origins.includes? "*"
end

Expand Down
3 changes: 3 additions & 0 deletions src/amber/router/cookies/store.cr
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ module Amber::Router::Cookies
expires : Time? = nil, domain : String? = nil,
secure : Bool = false, http_only : Bool = false,
extension : String? = nil)
name = URI.encode_www_form(name)
Copy link
Member

Choose a reason for hiding this comment

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

Is this a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

No this is preserving the existing expected behavior. I had to check the std lib, and 2 years ago the behavior for setting cookie names changed from implicitly encoding names and values to not encoding them at all. So I added this to preserve the expected behavior.

Now that I've said that I need to check and see if this means Amber will only work as expected on Crystal 1.0+

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah looks like this behavior changed as of 1.0 crystal-lang/crystal#10485

value = URI.encode_www_form(value)

if @cookies[name]? != value || expires
@cookies[name] = value
@set_cookies[name] = HTTP::Cookie.new(name, value, path, expires, domain, secure, http_only, extension)
Expand Down
2 changes: 1 addition & 1 deletion src/amber/router/session/abstract_store.cr
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Amber::Router::Session
abstract class AbstractStore
abstract def id
abstract def destroy
abstract def update(other_hash : Hash(String | Symbol, String))
abstract def update(hash : Hash(String | Symbol, String))
abstract def set_session
abstract def current_session
end
Expand Down
2 changes: 1 addition & 1 deletion src/amber/router/session/redis_store.cr
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ module Amber::Router::Session
end

def to_h
store.hgetall(session_id).each_slice(2).to_h
store.hgetall(session_id)
end

def update(hash : Hash(String | Symbol, String))
Expand Down
2 changes: 1 addition & 1 deletion src/amber/version.cr
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Amber
VERSION = "1.2.2"
VERSION = "1.3.0"
end
2 changes: 1 addition & 1 deletion src/amber/websockets/client_socket.cr
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ module Amber

def self.get_topic_channel(topic_path)
topic_channels = @@channels.select { |ch| WebSockets.topic_path(ch[:path]) == topic_path }
return topic_channels[0][:channel] if topic_channels.any?
return topic_channels[0][:channel] if !topic_channels.empty?
end

# Broadcast a message to all subscribers of the topic
Expand Down