-
Notifications
You must be signed in to change notification settings - Fork 22
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
Make config.yaml
optional
#286
Comments
I'm open to making a PR for this. It appears straightforward. |
I would suggest not doing this. The code in I'd suggest that an empty How about finding something to use But it's kind of likely that in some future you'll find a need for it. If a package includes more than one lint rule, then maybe it'd be nice if those are declared in a Or maybe, you decide that custom lints should implement a different interface, or they should put the lint implementation in If you decide to make such a change you'd have to make a breaking change. Unless, you add a version number in Of course, that might not be super relevant to you right now, because breaking changes probably happen relatively frequently due to the |
also, checking empty folders into Until recently |
Actually, in your case, if Loading all the custom lints just to print out what the names of the rules are is probably a bit slow. |
That's redundant. I can list lint rules and configure them without such a file.
An empty file doesn't work ;)
Sure but the directory is bound to have something it is. Otherwise what's the point of the plugin? |
I'll outline arguments worth considering as:
* (A) Most people will never write a custom_lint or devtools extension.
This is something very advanced users will do. We don't win much by making
`config.yaml` optional.
* (B) If we allow `extension/<pkg>/` to be empty, it could cause pain
because `git` doesn't like to commit empty-folders. And older versions of
pub doesn't like empty folders.
* (C) The `extension_discovery` package is already complicated to explain.
Do we really want to add another corner case? It's a lot to document. It's
more corner cases that developers interacting with `extension_discovery`
needs to understand.
* (D) It's kind of nice that there is room for meta-data. Most extension
systems will probably need it. Otherwise each extension system needs to
have its own config file and caching system. As evident from
extension_discovery this is not trivial to do. Efficiently loading
meta-data for each extension is not trivial. Load dart code is pretty slow,
compared to parsing a YAML file.
* (E) Forcing the act of providing an extension to be a very explicit
thing is good. We don't want arbitrary things to be found, it's better that
we don't find such things. Users having issues getting their extension
discovered are likely advances, and can probably figure out why.
* (F) Logic for detecting extensions currently stat the `config.yaml`
file, and looks at modification times. Now we'd have to also stat the
directory. This might not be bad, I just haven't investigated if there is
any cost to doing this.
I'll admit that most of my arguments sum up to: slightly more options is
slightly more complexity -- is that worth it?
Is there much cost to keeping a `config.yaml` file?
Sure but the directory is bound to have something it is. Otherwise what's
the point of the plugin?
You might have all the files implementing the plugin in `lib/src/`. Isn't
that what custom_lint does today?
I personally think that providing meta-data along with an extension is a
very natural thing. And that loading meta-data by compiling a program
containing all the extensions and then running it as a subprocess is pretty
slow. extension_discovery goes to great lengths to cache the meta-data and
ensuring its consistent after a new `pub get`. I guess the compiler also
does some caching, it's just not quite as fast (yet, hopefully one day).
…----
I can be convinced if others feel strongly. I just don't personally think
this is necessarily worth doing.
Is it really that bad?
On Wed, Jul 17, 2024, 21:47 Remi Rousselet ***@***.***> wrote:
Actually, in your case, if config.yaml listed all the rules that a
lint-package provides, then it'd possible to make a command that can
quickly list all the custom lint rules you can enable.
That's redundant. I can list lint rules and configure them without such a
file.
I truly have no use for the file.
The only thing this file would do for me is make things worse for package
authors by requiring extra steps.
I'd suggest that an empty config.yaml isn't the worst thing we could do.
An empty file doesn't work ;)
Due to how the package filters "null" configs (and an empty file will be
parsed as null), the config file has to be non-empty
also, checking empty folders into git can be a bit less fun. So you might
end up with a different empty file (like a .gitkeep file).
Sure but the directory is bound to have something it is. Otherwise what's
the point of the plugin?
It's just that the config.yaml is useless in our case.
—
Reply to this email directly, view it on GitHub
<#286 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABERZFWKAAGBNQA5NGEIVDZM3C3LAVCNFSM6AAAAABK7PA3XGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZUGEYTINZWHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hello!
I was working on a tool that needs plugins (custom_lint) and considered using extension_discovery. But in my case, the
config.yaml
is useless, as plugins will provide separate files inside (alib/main.dart
).Asking my users to specify a non-empty
config.yaml
just for the sake of it feels wrong.Could we make it optional?
The presence of that
extension/<name>
folder appears to be enough to detect plugins.The text was updated successfully, but these errors were encountered: