-
-
Notifications
You must be signed in to change notification settings - Fork 35
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 editorconfig-checker #430
Conversation
05d6928
to
f2fd3b7
Compare
main.sh
Outdated
@@ -69,6 +69,8 @@ download_and_extract() { | |||
case "${tool}" in | |||
# xbuild's binary name is "x", as opposed to the usual crate name | |||
xbuild) installed_bin=("${bin_dir}/x") ;; | |||
# editorconfig-checker's binary name is renamed below | |||
editorconfig-checker) installed_bin=("${bin_dir}/${tool}") ;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems editorconfig-checker binary is usually named ec
? https://github.com/editorconfig-checker/editorconfig-checker?tab=readme-ov-file#what https://github.com/editorconfig-checker/editorconfig-checker?tab=readme-ov-file#via-arguments
if so, this should be "${bin_dir}/ec{exe}".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go install github.com/editorconfig-checker/editorconfig-checker/v2/cmd/editorconfig-checker@latest
installs editorconfig-checker
homebrew, nix and arch use editor-checker
https://repology.org/project/editorconfig-checker/versions
I'm not sure about alpine. But in general distros are unlikely to let an obscure tool like this have a two letter bin name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the info. Agreed that "editorconfig-checker" is a reasonable binary name here.
alpine: "package name" is "editorconfig-checker", but "binary name" is "ec": https://pkgs.alpinelinux.org/contents?branch=v3.14&name=editorconfig%2dchecker&arch=x86_64&repo=community
arch: "package name" is "editorconfig-checker", but "binary names" are both "ec" and "editor-checker": https://archlinux.org/packages/extra/x86_64/editorconfig-checker/ (click "Package Contents" section to show list)
homebrew: both "package name" and "binary names" are "editorconfig-checker": https://github.com/Homebrew/homebrew-core/blob/675ca659ebe69e0829d59104c2b74089e0acf6f6/Formula/e/editorconfig-checker.rb#L31
nix:
- NixOS/nixpkgs: IIUC both "package name" and "binary names" are "editorconfig-checker": https://github.com/NixOS/nixpkgs/blob/release-23.11/pkgs/development/tools/misc/editorconfig-checker/default.nix
- editorconfig-checker/editorconfig-checker/default.nix: "package name" is "editorconfig-checker", but "binary names" are both "ec" and "editor-checker" (ec is symlink): https://github.com/editorconfig-checker/editorconfig-checker/blob/0e32afcec97ba925d3c5f6530186159e3f37bcb0/default.nix#L19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also is looks like they are going to discontinue using ec
editorconfig-checker/editorconfig-checker#255
DEVELOPMENT.md
Outdated
@@ -23,6 +23,18 @@ See JSON files in `tools/codegen/base` directory for examples of the manifest. | |||
> GITHUB_TOKEN=$(gh auth status --show-token 2>&1 | sed -n 's/^.*Token: \(.*\)$/\1/p') ./tools/manifest.sh <tool> | |||
> ``` | |||
|
|||
## Local testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main.sh
is a script that is likely to change contants outside the repository, so I don't think it should be run locally, basically, although it would be fine if you are testing it within an isolated environment, such as a container.
Anyway, I don't want to require contributors to run tests locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I've removed those additions
Co-authored-by: Taiki Endo <te316e89@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Published in 2.32.0. |
Add https://github.com/editorconfig-checker/editorconfig-checker