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

Custom check shipped with lib not loading #1068

Closed
epinault opened this issue Aug 25, 2023 · 14 comments
Closed

Custom check shipped with lib not loading #1068

epinault opened this issue Aug 25, 2023 · 14 comments

Comments

@epinault
Copy link

Environment

  • Credo version (mix credo -v): 1.7.x
  • Erlang/Elixir version (elixir -v): 1.14.x
  • Operating system: Ventura

What were you trying to do?

I creating a credo plugin (that is really close to the example), and trying to ship also custom check as part of the plug into the same package
Then I am trying to use the plugin and that custom check in an application. I was hoping to define the new check as part of the plugin to load so I don t have to define anything in the application

So say I have MyApp as application, and my Plugin is a separate package called MyCredoPlugin
MyCredoPlugin .credo.exs file has this

%{
  #
  # You can have as many configs as you like in the `configs:` field.
  configs: [
    %{
      name: "default",
      requires: ["./lib/credo_podium/check/**/*.ex"],
      checks: [
        #without this line, it s does not seem to be running from the plugin config
        {Credo.Check.Refactor.ABCSize, []},
        ## this line does not seem to still enable the check in this plugin when running credo
        {CredoPodium.Check.Security.LogWithInspect, []}
      ]
    }
  ]
}

Expected outcome

I would expect the plugin to be enabled and running by default the new check. and see bunch of output that I know MyApp will trigger

Actual outcome

it s not running the check though it is loading the plugin correctly.

In order the enable that custom check. I had to add it to my .credo.exs inside the MyApp folder as such

%{
  #
  # You can have as many configs as you like in the `configs:` field.
  configs: [
    %{
      name: "default",
      plugins: [{MyCredoPlugin, []}],
      checks: [
        #without this line, it s does not seem to be running from the plugin config
        {CredoPodium.Check.Security.LogWithInspect, []}
      ]
    }
  ]
}

By the way I will likely send an MR for this check if you are interested in it.

@rrrene
Copy link
Owner

rrrene commented Aug 25, 2023

Here is a sample project which loads a demo plugin which brings its own check.

I think this demonstrates what you are trying to do, right?

If you are still having problems, can you create a minimal reproducible example?

@epinault
Copy link
Author

yea this assumes that it s all in the same repo that example. I ll see if i can build a minimal repro next week

@rrrene
Copy link
Owner

rrrene commented Aug 25, 2023

yea this assumes that it s all in the same repo that example.

So, you have (a) a plugin in one repo, (b) a check in another repo and (c) a project where this should come together?

@epinault
Copy link
Author

I have a lib that define the plugin and includes custom checks
then I have an app that use that lib to configure credo and run the check defined by that lib

@rrrene
Copy link
Owner

rrrene commented Aug 25, 2023

And how is this different from the example linked above?

@epinault
Copy link
Author

epinault commented Aug 25, 2023

so I guess the setup is similar but I don t want the sample project to load the custom check. I put the require and the custom check in the demo plugin. So I was hoping not to have to have every application to have to enable the custom check in their credo file.

so I would say try to move the require and the custom check enabling in the plugin instead and that should about what I have

@rrrene
Copy link
Owner

rrrene commented Aug 27, 2023

So I was hoping not to have to have every application to have to enable the custom check in their credo file.

And you don't have to (again: see the linked example above).

I would very much like to help you (again: can you put something online that reproduces this error, so I can look at it?).

@epinault
Copy link
Author

there you go!

clone both https://github.com/epinault/credo_sample_project and https://github.com/epinault/credo_demo_plugin
both in the same dir

if you run mix credo,, you ll noticed I added a debug that show it s never running the Better check

@rrrene
Copy link
Owner

rrrene commented Aug 28, 2023

If I run mix credo suggest I get this:

image

Do you get another result?

@epinault
Copy link
Author

epinault commented Aug 28, 2023

❯ elixir -v
Erlang/OTP 25 [erts-13.2.2.1] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]

Elixir 1.14.5 (compiled with Erlang/OTP 24)

❯ mix credo --help
Credo Version 1.7.0-ref.master.7179615+uncommittedchanges
Usage: $ mix credo <command> [options]

credo_sample_project on  master is 📦 v0.1.0 via 💧 on ☁️  (us-west-2) took 2s
❯ mix credo
By the power of !
```


only difference I see is you use suggest?  I have it all over my CI or running locally as this
```
mix credo --strict
```

and now i see this error

❯ mix credo --strict
** (credo) Unknown switch for `demo` command: --strict


yet the lock file says


```
  "credo": {:hex, :credo, "1.7.0", "6119bee47272e85995598ee04f2ebbed3e947678dee048d10b5feca139435f75", [:mix], [{:bunt, "~> 0.2.1", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "6839fcf63d1f0d1c0f450abc8564a57c43d644077ab96f2934563e68b8a769d7"},
  ```

@rrrene
Copy link
Owner

rrrene commented Aug 28, 2023

The demo plugin teaches you to

  1. add a config (for a check provided by the plugin)
  2. register a new command
  3. register a CLI switch to configure that command and
  4. set that command as the "default command"

You seem to have used that demo plugin, wanting to use the first point and have not stripped out the code responsible for the last three points.

Therefore, when you type mix credo, it does not do what you think.

Did you never think "Why is it printing 'By the power of !'??" ...

@epinault
Copy link
Author

I did not realize this plugin was also doing another command. I ll modify once more the code to reflect more my project . let me fully get a commit for those repo that reflect and remove that CLI command

@epinault
Copy link
Author

meanwhile would you mind looking at #1069 since really that would solving my problem if that was part of credo already anyway

@epinault
Copy link
Author

epinault commented Aug 28, 2023

ok so I can see it working . I am not sure why it was not working on mh other project but once I clobber my plugin package then it started to behave again correctly. Though both were using the exact same plugin and using path: to point to it.. I wonder if there is an issue around hex and some caching using path at time.

I literally had the same config between those project another one. and only thing that made it work was doing a

mix deps.clean my_plugin

@epinault epinault closed this as completed Sep 5, 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

No branches or pull requests

2 participants