From 034911d5ae9ac7b07c094eadcc39af902a5a6ee5 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Sun, 25 Oct 2026 02:56:05 +0200 Subject: [PATCH 1/2] Add Ameba to the dev image and fix its issues Ameba checks Crystal code for common mistakes and style problems. Since v1.7.0 it ships as a static binary, so the dev image downloads it instead of adding a shard dependency. Migration.from_io stays over the complexity limit because of the support for deprecated markers. An inline exception marks it until that support goes away. --- .../unreleased/internal-20261025-012014.yaml | 5 +++ AGENTS.md | 10 ++++- compose.yaml | 6 ++- docker/dev.dockerfile | 37 ++++++++++++++++ src/drift/commands/new.cr | 6 +-- src/drift/commands/status.cr | 2 +- src/drift/migration.cr | 43 +++++++++++-------- 7 files changed, 84 insertions(+), 25 deletions(-) create mode 100644 .changes/unreleased/internal-20261025-012014.yaml create mode 100644 docker/dev.dockerfile diff --git a/.changes/unreleased/internal-20261025-012014.yaml b/.changes/unreleased/internal-20261025-012014.yaml new file mode 100644 index 0000000..cba446d --- /dev/null +++ b/.changes/unreleased/internal-20261025-012014.yaml @@ -0,0 +1,5 @@ +kind: internal +body: Lint code with Ameba in CI and the dev container +time: 2026-10-25T01:20:14.000000+02:00 +custom: + Issue: "" diff --git a/AGENTS.md b/AGENTS.md index 734d645..2a22afe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -67,8 +67,14 @@ crystal spec --verbose # Verbose # Format (enforced by CI) crystal tool format src/ spec/ # Apply crystal tool format --check src/ spec/ # Check only + +# Lint (enforced by CI) +ameba # Check src/ and spec/ ``` +Ameba is installed in the dev image (`docker/dev.dockerfile`). After a change +to that file, run `docker compose build app`. + ## Pull request guidelines **Branch naming:** Use dashes, not slashes (`feature-add-postgres-support`). @@ -85,11 +91,11 @@ Kinds: `added`, `improved`, `changed`, `deprecated`, `removed`, `fixed`, `securi **Before submitting:** 1. Tests pass -2. Code formatted +2. Code formatted and `ameba` passes 3. Changelog entry added 4. Rebased on main -**CI checks:** Format, tests, changelog. +**CI checks:** Format, lint, tests, changelog. ## Contribution policy diff --git a/compose.yaml b/compose.yaml index 3f5f793..d366f14 100644 --- a/compose.yaml +++ b/compose.yaml @@ -1,6 +1,10 @@ services: app: - image: ghcr.io/luislavena/hydrofoil-crystal:${CRYSTAL_VERSION:-1.16} + build: + context: . + dockerfile: docker/dev.dockerfile + args: + CRYSTAL_VERSION: ${CRYSTAL_VERSION:-1.16} command: overmind start -f Procfile.dev working_dir: /workspace/${COMPOSE_PROJECT_NAME} environment: diff --git a/docker/dev.dockerfile b/docker/dev.dockerfile new file mode 100644 index 0000000..d5c0c76 --- /dev/null +++ b/docker/dev.dockerfile @@ -0,0 +1,37 @@ +# syntax=docker/dockerfile:1 + +# --- +# Development image: hydrofoil-crystal plus project-specific tools +ARG CRYSTAL_VERSION=1.16 +FROM ghcr.io/luislavena/hydrofoil-crystal:${CRYSTAL_VERSION} + +# install development utilities +RUN --mount=type=tmpfs,target=/tmp \ + set -eux; \ + cd /tmp; \ + # Ameba + { \ + export AMEBA_VERSION=1.7.0; \ + case "$(arch)" in \ + x86_64) \ + export \ + AMEBA_ARCH=x86_64 \ + AMEBA_SHA256=7ed747711e76579902c5a4165a78a41b1e5acf0681ffcc538197bab435b4318f \ + ; \ + ;; \ + aarch64) \ + export \ + AMEBA_ARCH=aarch64 \ + AMEBA_SHA256=6467cdf69cae19ab3c52be93d2a1604e9b50e67fc20c7c1cb257117bd970837c \ + ; \ + ;; \ + esac; \ + curl --fail -Lo ameba.tar.gz https://github.com/crystal-ameba/ameba/releases/download/v${AMEBA_VERSION}/ameba-${AMEBA_VERSION}-linux-musl-${AMEBA_ARCH}.tar.gz; \ + echo "${AMEBA_SHA256} *ameba.tar.gz" | sha256sum -c - >/dev/null 2>&1; \ + tar -xf ameba.tar.gz; \ + mv ameba /usr/local/bin/; \ + rm -f ameba.tar.gz; \ + }; \ + # smoke tests + [ "$(command -v ameba)" = '/usr/local/bin/ameba' ]; \ + ameba --version diff --git a/src/drift/commands/new.cr b/src/drift/commands/new.cr index b353f7d..14c5cba 100644 --- a/src/drift/commands/new.cr +++ b/src/drift/commands/new.cr @@ -18,10 +18,10 @@ module Drift module Commands class New < Command MIGRATION_TEMPLATE = <<-SQL - -- drift:up + -- drift:up - -- drift:down - SQL + -- drift:down + SQL def run(*args) migration_name = args.first? diff --git a/src/drift/commands/status.cr b/src/drift/commands/status.cr index 04798c1..6d8171d 100644 --- a/src/drift/commands/status.cr +++ b/src/drift/commands/status.cr @@ -49,7 +49,7 @@ module Drift cell id.to_s end - if migration = applied.find { |m| m.id == id } + if migration = applied.find { |entry| entry.id == id } cell "Yes" cell migration.batch.to_s, align: :right cell migration.applied_at.to_s diff --git a/src/drift/migration.cr b/src/drift/migration.cr index 61e23be..a80ba44 100644 --- a/src/drift/migration.cr +++ b/src/drift/migration.cr @@ -23,15 +23,18 @@ module Drift # :nodoc: MAGIC_MARKER = "-- drift:" + # :nodoc: + DEPRECATED_MARKERS = {"migrate" => "up", "rollback" => "down"} + getter id : Int64 getter filename : String? = nil @statements : Hash(Type, Array(String)) def initialize(@id : Int64, @filename : String? = nil) - @statements = Type.values.to_h { |type| + @statements = Type.values.to_h do |type| {type, Array(String).new} - } + end end def add(type : Type, statement : String) @@ -48,39 +51,34 @@ module Drift @statements[type] end + # TODO: remove ameba:disable along with the deprecated markers support + # ameba:disable Metrics/CyclomaticComplexity def self.from_io(io, id : Int64, filename : String? = nil) : self migration = new(id, filename) buffer = IO::Memory.new type = nil multi_statement_mode = false - deprecated_warning_shown = false + deprecated_marker_found = false io.each_line do |line| stripped_line = line.strip # detect markers if stripped_line.starts_with?(MAGIC_MARKER) - case stripped_line[MAGIC_MARKER.size..-1] + raw_marker = stripped_line[MAGIC_MARKER.size..-1] + marker, deprecated = normalize_marker(raw_marker) + if deprecated && !deprecated_marker_found + STDERR.puts "DEPRECATED: 'drift:#{raw_marker}' marker in '#{filename || id}' is deprecated, use 'drift:#{marker}' instead" + deprecated_marker_found = true + end + + case marker when "up" type = Type::Up next when "down" type = Type::Down next - when "migrate" - unless deprecated_warning_shown - STDERR.puts "DEPRECATED: 'drift:migrate' marker in '#{filename || id}' is deprecated, use 'drift:up' instead" - deprecated_warning_shown = true - end - type = Type::Up - next - when "rollback" - unless deprecated_warning_shown - STDERR.puts "DEPRECATED: 'drift:rollback' marker in '#{filename || id}' is deprecated, use 'drift:down' instead" - deprecated_warning_shown = true - end - type = Type::Down - next when "begin" multi_statement_mode = true next @@ -113,6 +111,15 @@ module Drift migration end + # Returns the current name for a marker, and whether it was deprecated + private def self.normalize_marker(marker : String) : {String, Bool} + if replacement = DEPRECATED_MARKERS[marker]? + {replacement, true} + else + {marker, false} + end + end + def self.load_file(filename : String) : self basename = File.basename(filename) From 247bd44ee953e76bccfb69f3f4af72bbc42779fc Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Sun, 25 Oct 2026 02:56:17 +0200 Subject: [PATCH 2/2] Run Ameba in CI The official Ameba action reports issues inline on pull requests. Its image follows the latest Ameba release, so the version input limits the rules to 1.7.0, the same as the dev image. --- .github/workflows/test.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 62c4bb1..f42e1c7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -27,6 +27,15 @@ jobs: - uses: crystal-lang/install-crystal@v1 - run: crystal tool format --check + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: crystal-ameba/github-action@v1 + with: + # keep rules in sync with docker/dev.dockerfile + version: 1.7.0 + test: runs-on: ubuntu-latest steps: