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

Add GitHub CI #6

Merged
merged 1 commit into from
Feb 7, 2025
Merged

Add GitHub CI #6

merged 1 commit into from
Feb 7, 2025

Conversation

kianmeng
Copy link
Contributor

@kianmeng kianmeng commented Feb 6, 2025

List of changes:

  • ignore files from mix format
  • remove unused deps
  • resolve optional deps that failed the test

Enum.flat_map(
["{mix,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}"],
&Path.wildcard(&1, match_dot: true)
) -- ["test/fixtures/example.exs"]
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mix format failed for file: test/fixtures/example.exs
** (SyntaxError) invalid syntax found on test/fixtures/example.exs:8:18:
    error: invalid character "o" after number 0. If you intended to write a number, make sure to separate the number from the character (using comma, space, etc). If you meant to write a function name or a variable, note that identifiers in Elixir cannot start with numbers. Unexpected token: o
    │
  8 │ 0b012 ; 0xboar ; 0o888
    │                  ^
    │
    └─ test/fixtures/example.exs:8:18
    (elixir 1.18.2) lib/code.ex:1031: Code.format_string!/2
    (mix 1.18.2) lib/mix/tasks/format.ex:6[9](https://github.com/kianmeng/makeup_eex/actions/runs/13182353912/job/36796043369#step:5:10)6: Mix.Tasks.Format.elixir_format/2
    (mix 1.18.2) lib/mix/tasks/format.ex:715: Mix.Tasks.Format.format_file/2
    (elixir 1.18.2) lib/task/supervised.ex:[10](https://github.com/kianmeng/makeup_eex/actions/runs/13182353912/job/36796043369#step:5:11)1: Task.Supervised.invoke_mfa/2
    (elixir 1.18.2) lib/task/supervised.ex:36: Task.Supervised.reply/4

@@ -1,7 +1,4 @@
%{
"benchee": {:hex, :benchee, "1.0.1", "66b211f9bfd84bd97e6d1beaddf8fc2312aaabe192f776e8931cb0c16f53a521", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}], "hexpm", "3ad58ae787e9c7c94dd7ceda3b587ec2c64604563e049b2a0e8baafae832addb"},
"benchee_markdown": {:hex, :benchee_markdown, "0.2.7", "2b3967a8c49acb4a1fd4a5dc42fe3fc6d9713154dad4c9053b07997a951ec6c4", [:mix], [{:benchee, ">= 0.99.0 and < 2.0.0", [hex: :benchee, repo: "hexpm", optional: false]}], "hexpm", "c2ed04e0c8f7fb4aa75c3006973e14cca93adf2bf61713e421530dee63368b12"},
"deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Run mix deps.get && mix deps.unlock --check-unused
Resolving Hex dependencies...
Resolution completed in 0.023s
Unchanged:
  earmark_parser 1.4.41
  ex_doc 0.34.2
  makeup 1.2.1
  makeup_elixir 1.0.0
  makeup_erlang 1.0.1
  makeup_html 0.1.1
  nimble_parsec 1.4.0
* Getting ex_doc (Hex package)
* Getting earmark_parser (Hex package)
* Getting makeup_erlang (Hex package)
** (Mix) Unused dependencies in mix.lock file:

  * :benchee
  * :benchee_markdown
  * :deep_merge

mix.exs Outdated
@@ -41,7 +41,7 @@ defmodule MakeupEEx.MixProject do
{:nimble_parsec, "~> 1.2"},
# Sub-languages
{:makeup_elixir, "~> 1.0"},
{:makeup_html, "~> 0.1.0 or ~> 1.0", optional: true},
{:makeup_html, "~> 0.1.0 or ~> 1.0"},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Run mix test
Compiling 6 files (.ex)
Generated makeup_eex app


  1) test HEEx inside attribute value (the EEx splits the string) (Makeup.Lexers.HEExLexerTest)
Error:      test/makeup/lexers/heex_lexer_test.exs:35
     ** (RuntimeError) The HEEx / EEx+HTML lexer requires an HTML lexer to be registered. You can do this for example by including
     
         {:makeup_html, "~> 1.0"}
     
     in your project's dependencies.
     
     code: assert lex_heex(~S[<span class="my-<%= @class %>">]) == [
     stacktrace:
       (makeup_eex 2.0.0) lib/makeup_eex.ex:12: MakeupEEx.dynamic_html_lexer/0
       (makeup_eex 2.0.0) lib/makeup/lexers/heex_lexer.ex:290: Makeup.Lexers.HEExLexer.lex/2
       (makeup_eex 2.0.0) lib/makeup/lexers/eex_lexer/testing.ex:5[8](https://github.com/kianmeng/makeup_eex/actions/runs/13182353912/job/36796042560#step:9:9): Makeup.Lexers.EExLexer.Testing.lex_heex/1
       test/makeup/lexers/heex_lexer_test.exs:36: (test)

Copy link
Contributor

Choose a reason for hiding this comment

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

That's weird. The tests pass locally for me 🤔

Copy link
Contributor Author

@kianmeng kianmeng Feb 6, 2025

Choose a reason for hiding this comment

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

It works in local dev (1.18, 27, ubuntu 24.04) for me as well, not in CI job, test (1.12, 22, ubuntu-20.04). See https://github.com/kianmeng/makeup_eex/actions/runs/13182353912/job/36796042560

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we bump the minimum Elixir version?

Copy link
Contributor Author

@kianmeng kianmeng Feb 7, 2025

Choose a reason for hiding this comment

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

Same with test (1.13, 22, ubuntu-20.04) if we make makeup_html an optional deps, {:makeup_html, "~> 0.1.0 or ~> 1.0", optional: true},:

See https://github.com/kianmeng/makeup_eex/actions/runs/13191664881/job/36825616496

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I know what it is. Earlier Elixir versions did not start optional dependencies automatically. So we need to either bump the Elixir version more or add Application.ensure_all_started(:makeup_html) to test/test_helper.exs.

List of changes:
- ignore files from mix format
- remove unused deps
- resolve optional deps that failed the test
@josevalim josevalim merged commit 6ef9107 into elixir-makeup:main Feb 7, 2025
@josevalim
Copy link
Contributor

💚 💙 💜 💛 ❤️

@kianmeng
Copy link
Contributor Author

kianmeng commented Feb 7, 2025

🥳 🥳 🥳 🥳 🥳

@josevalim
Copy link
Contributor

@kianmeng could you kindly send a PR for adding CI to makeup_json and makeup_c?

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.

2 participants