exercism / org-wide-files

MIT License
7 stars 8 forks source link

consider adding `fetch-configlet` #262

Closed ee7 closed 1 year ago

ee7 commented 2 years ago

Follow-up from https://github.com/exercism/elixir/pull/1218#issuecomment-1285847533 and https://github.com/exercism/elixir/pull/1217#issuecomment-1285855493

Let's try to:

kytrinyx commented 2 years ago

I looked at the repositories that have a fetch-configlet file with a different filename.

The Elixir folks have confirmed that they're happy to switch to whatever is the org-wide standard, and mentioned a slight preference for having an explicit file extension (.sh).

I'm pretty sure that I'm the one who chose the original filename (hyphenated, no extension). This choice was made lightly, with little thought to alternatives. I simply used what was habitual for me.

The upstream repository has both a fetch-configlet script and a fetch-configlet.ps1 script. I think both should be added to the org-wide-files, and in order to be explicit, I think it would make sense to name the first one fetch-configlet.sh.

ee7 commented 2 years ago

Now that I think about it again, moving the fetch-configlet scripts to the org-wide-files is a bit annoying for configlet developers because:

Without thinking too hard, maybe my preferences are:

  1. Keep configlet as the upstream repo for the scripts, and allow org-wide-files to sync files that live in a different repo.
  2. Make org-wide-files the upstream repo for the scripts, and remove the scripts from the configlet repo. Deal with not being able to see changes to the scripts in the same repo, and needing to commit to a separate repo to reflect certain changes in the configlet repo.
  3. Same as 2, but keep the scripts in the configlet repo as a downstream (make org-wide-files target the configlet repo).

The fetch-configlet scripts are clearly files that we want in many repos, with a single source of truth. So it does make sense for them to live in the repo called "org-wide-files". If option 1 is too hard or strange then I'm OK with option 2. Option 3 might be a little confusing. Thoughts? Can we see any other options?

And in the time since we created org-wide-files, did GitHub add new mechanisms for synchronizing changes across multiple repos?

Edit: some not-especially-useful discussion from before we'd set up org-wide-files: https://github.com/exercism/configlet/issues/310


I'm pretty sure that I'm the one who chose the original filename (hyphenated, no extension). This choice was made lightly, with little thought to alternatives.

I don't care that much about the file extension, but to give one point of view, the Google Shell Style Guide says:

Executables should have no extension (strongly preferred) or a .sh extension. Libraries must have a .sh extension and should not be executable.

It is not necessary to know what language a program is written in when executing it and shell doesn’t require an extension so we prefer not to use one for executables.

However, for libraries it’s important to know what language it is and sometimes there’s a need to have similar libraries in different languages. This allows library files with identical purposes but different languages to be identically named except for the language-specific suffix.

And from https://mywiki.wooledge.org/FullBashGuide:

please refrain from giving scripts a .sh extension. It serves no purpose, and it's completely misleading (since it's going to be a bash script, not an sh script).

And the fetch-configlet script is indeed a bash script, not a POSIX sh script.

However, from koalaman/shellcheck/issues#1369 (comment) bash shebang with .sh file extension seems common.

kytrinyx commented 2 years ago

That's good to know for the extension. I think it makes sense to leave it off, in that case.

The fetch-configlet scripts are clearly files that we want in many repos, with a single source of truth.

It just occurred to me that we don't actually want them in all the repositories. Just the track repositories.

How about having Configlet do it? Configlet is also only in the track repositories.

ee7 commented 2 years ago

It just occurred to me that we don't actually want them in all the repositories. Just the track repositories.

The org-wide-files repo does have a concept of global files, tooling-specific files, and track-specific files:

How about having Configlet do it? Configlet is also only in the track repositories.

Could you elaborate? Do you mean:

ee7 commented 2 years ago

Here's some more past discussion, in case it's useful:

We did decide against "make the fetch-configlet script in track repos download and execute the latest fetch script" at one point (and I'd still be against that option):

kytrinyx commented 2 years ago

I would prefer the fetch-configlet scripts to live in the configlet repository. But I would also prefer org-wide-files to do the syncing, especially since it can support tracks-files.

I think my preference would be to see if we can make org-wide-files get the file from the configlet repository without that turning into a mess.

ee7 commented 2 years ago

Great. Sounds like we're on the same page here.

I've reminded myself that org-wide-files does not currently target exercism/request-new-language-track in PRs for tracks-files:

Maybe it could.

kytrinyx commented 2 years ago

It would make sense to target it. That way newly bootstrapped repositories would be fully up-to-date.

ErikSchierboom commented 2 years ago

Yeah that makes sense. Could you create an issue?

ee7 commented 2 years ago

Could you create an issue?

Done: https://github.com/exercism/org-wide-files/issues/263

ee7 commented 1 year ago

I would prefer the fetch-configlet scripts to live in the configlet repository. But I would also prefer org-wide-files to do the syncing, especially since it can support tracks-files.

I think my preference would be to see if we can make org-wide-files get the file from the configlet repository without that turning into a mess.

@kytrinyx and @ErikSchierboom - any idea how easy this is?

There's a bunch of fetch-configlet script changes in the pipeline:

so it could be nice to get org-wide-files to handle the mass PRs.

If it's not easy, what do we think about just duplicating the scripts for now? That is: keep fetch-configlet scripts in the configlet repo, add them to org-wide-files too, and update them in org-wide-files when we want to distribute the changes?

Note that in the time since my posts above I've added testing of both bash and PowerShell fetch-configlet scripts to CI in the configlet repo (see https://github.com/exercism/configlet/commit/0be1889317f22196a2a5cafa2f8f5179a84faa87).

Also, should we add the PowerShell script to org-wide-files? I lean towards "no", but we do need to update it on every track that it exists (for the same upcoming asset renaming). I'm not a PowerShell user, so I don't want to commit to maintaining it, and it will need more updates in the future (see open issues for fetch-configlet).

At the time of writing, the fetch-configlet.ps1 script exists in these 25 (of the 92) track repos:

And of those, only 13 are active tracks:

It was added to request-new-language-track in https://github.com/exercism/request-new-language-track/commit/7b144840cf838df361a928c9144fd9db3add672f.

ErikSchierboom commented 1 year ago

If it's not easy, what do we think about just duplicating the scripts for now? That is: keep fetch-configlet scripts in the configlet repo, add them to org-wide-files too, and update them in org-wide-files when we want to distribute the changes?

I'm fine with duplicating them.

Also, should we add the PowerShell script to org-wide-files? I lean towards "no", but we do need to update it on every track that it exists (for the same upcoming asset renaming). I'm not a PowerShell user, so I don't want to commit to maintaining it, and it will need more updates in the future (see open issues for fetch-configlet).

Well, I think it would make sense to have them sync too, but only for the tracks interested in them. We could maybe use the org-wide-files-config.toml file that we also used to enable or disable format checking (see https://github.com/exercism/julia/blob/main/.github/org-wide-files-config.toml).

kytrinyx commented 1 year ago

I believe the ps1 script is in all tracks at the moment (I made the incorrect assumption that this was the desired behavior, so I added them in the last batch sync PRs).

This script would be used by any contributor/maintainer who is developing on a Windows machine, right?

ee7 commented 1 year ago

I believe the ps1 script is in all tracks at the moment

Incorrect, right? As far as I'm aware, what I wrote at the bottom of https://github.com/exercism/org-wide-files/issues/262#issuecomment-1304634330 is accurate. I think it's just tracks that were bootstrapped after 2020-10-09, plus a tiny few where maintainers added it manually (exactly cpp, csharp, dart, fsharp, javascript, typescript).

$ cd my-dir-with-every-exercism-track
$ find . -name 'fetch-configlet.ps1' | sort
./8th/bin/fetch-configlet.ps1
./abap/bin/fetch-configlet.ps1
./awk/bin/fetch-configlet.ps1
./bqn/bin/fetch-configlet.ps1
./chapel/bin/fetch-configlet.ps1
./cobol/bin/fetch-configlet.ps1
./cpp/bin/fetch-configlet.ps1
./csharp/bin/fetch-configlet.ps1
./dart/bin/fetch-configlet.ps1
./free-pascal/bin/fetch-configlet.ps1
./fsharp/bin/fetch-configlet.ps1
./gnucobol/bin/fetch-configlet.ps1
./javascript/bin/fetch-configlet.ps1
./jq/bin/fetch-configlet.ps1
./openeuphoria/bin/fetch-configlet.ps1
./qsharp/bin/fetch-configlet.ps1
./red/bin/fetch-configlet.ps1
./solidity/bin/fetch-configlet.ps1
./typescript/bin/fetch-configlet.ps1
./unison/bin/fetch-configlet.ps1
./vlang/bin/fetch-configlet.ps1
./wasm/bin/fetch-configlet.ps1
./wren/bin/fetch-configlet.ps1
./z3/bin/fetch-configlet.ps1
./zig/bin/fetch-configlet.ps1

Listing the Exercism track repos by creation date:

gh repo list exercism --limit 300 --topic exercism-track \
    --visibility public --no-archived \
    --json createdAt,name --jq 'sort_by(.createdAt)' \
    | jq
JSON output ```json [ { "createdAt": "2014-02-27T18:11:43Z", "name": "ruby" }, { "createdAt": "2014-02-28T00:45:13Z", "name": "java" }, { "createdAt": "2014-02-28T01:02:22Z", "name": "coffeescript" }, { "createdAt": "2014-02-28T01:19:15Z", "name": "elixir" }, { "createdAt": "2014-02-28T02:28:52Z", "name": "clojure" }, { "createdAt": "2014-02-28T02:51:19Z", "name": "go" }, { "createdAt": "2014-02-28T02:59:12Z", "name": "haskell" }, { "createdAt": "2014-02-28T03:04:58Z", "name": "objective-c" }, { "createdAt": "2014-02-28T03:23:52Z", "name": "ocaml" }, { "createdAt": "2014-02-28T03:35:06Z", "name": "perl5" }, { "createdAt": "2014-02-28T03:48:58Z", "name": "python" }, { "createdAt": "2014-02-28T03:57:36Z", "name": "scala" }, { "createdAt": "2014-02-28T05:23:39Z", "name": "erlang" }, { "createdAt": "2014-02-28T05:29:55Z", "name": "common-lisp" }, { "createdAt": "2014-02-28T05:31:39Z", "name": "php" }, { "createdAt": "2014-02-28T05:33:52Z", "name": "rust" }, { "createdAt": "2014-02-28T09:24:11Z", "name": "csharp" }, { "createdAt": "2014-02-28T18:14:29Z", "name": "raku" }, { "createdAt": "2014-03-17T02:12:04Z", "name": "cpp" }, { "createdAt": "2014-03-24T14:55:04Z", "name": "d" }, { "createdAt": "2014-04-17T15:24:48Z", "name": "javascript" }, { "createdAt": "2014-05-02T17:08:31Z", "name": "fsharp" }, { "createdAt": "2014-06-02T17:52:31Z", "name": "lua" }, { "createdAt": "2014-06-10T15:04:28Z", "name": "swift" }, { "createdAt": "2014-07-18T03:45:51Z", "name": "nim" }, { "createdAt": "2014-08-23T18:07:37Z", "name": "bash" }, { "createdAt": "2014-09-23T07:38:30Z", "name": "powershell" }, { "createdAt": "2014-09-24T05:22:10Z", "name": "r" }, { "createdAt": "2014-09-25T04:07:05Z", "name": "c" }, { "createdAt": "2014-09-27T15:15:18Z", "name": "sml" }, { "createdAt": "2014-09-30T03:02:11Z", "name": "vbnet" }, { "createdAt": "2014-10-05T21:25:35Z", "name": "groovy" }, { "createdAt": "2015-01-28T15:03:11Z", "name": "plsql" }, { "createdAt": "2015-01-30T04:46:03Z", "name": "scheme" }, { "createdAt": "2015-04-23T02:42:35Z", "name": "cfml" }, { "createdAt": "2015-06-05T02:26:46Z", "name": "emacs-lisp" }, { "createdAt": "2015-07-19T00:13:52Z", "name": "haxe" }, { "createdAt": "2015-07-19T16:25:19Z", "name": "pony" }, { "createdAt": "2015-07-21T00:54:39Z", "name": "lfe" }, { "createdAt": "2015-08-18T00:11:42Z", "name": "racket" }, { "createdAt": "2015-09-08T19:04:12Z", "name": "elm" }, { "createdAt": "2015-12-09T07:31:10Z", "name": "kotlin" }, { "createdAt": "2016-03-08T15:26:13Z", "name": "crystal" }, { "createdAt": "2016-05-31T14:32:17Z", "name": "factor" }, { "createdAt": "2016-06-27T21:02:37Z", "name": "idris" }, { "createdAt": "2016-07-05T21:31:06Z", "name": "purescript" }, { "createdAt": "2016-07-09T13:30:01Z", "name": "ceylon" }, { "createdAt": "2016-08-18T23:46:03Z", "name": "mips" }, { "createdAt": "2016-10-03T14:34:42Z", "name": "delphi" }, { "createdAt": "2016-10-04T00:09:21Z", "name": "prolog" }, { "createdAt": "2016-11-21T01:58:59Z", "name": "julia" }, { "createdAt": "2017-02-12T21:51:23Z", "name": "typescript" }, { "createdAt": "2017-02-20T22:12:38Z", "name": "vimscript" }, { "createdAt": "2017-05-05T17:48:45Z", "name": "coq" }, { "createdAt": "2017-05-29T21:20:00Z", "name": "fortran" }, { "createdAt": "2017-06-30T13:47:50Z", "name": "dart" }, { "createdAt": "2017-08-10T09:47:14Z", "name": "gnu-apl" }, { "createdAt": "2018-06-06T14:47:42Z", "name": "reasonml" }, { "createdAt": "2018-06-10T17:25:36Z", "name": "pharo-smalltalk" }, { "createdAt": "2018-06-13T14:38:37Z", "name": "ballerina" }, { "createdAt": "2018-12-01T16:22:22Z", "name": "tcl" }, { "createdAt": "2019-02-26T00:37:43Z", "name": "ada" }, { "createdAt": "2019-02-26T00:46:47Z", "name": "x86-64-assembly" }, { "createdAt": "2019-02-26T00:49:55Z", "name": "shen" }, { "createdAt": "2019-07-16T21:12:29Z", "name": "gleam" }, { "createdAt": "2019-07-16T21:16:23Z", "name": "system-verilog" }, { "createdAt": "2019-11-18T17:53:21Z", "name": "arm64-assembly" }, { "createdAt": "2020-03-26T14:22:03Z", "name": "babashka" }, { "createdAt": "2020-03-26T14:43:27Z", "name": "j" }, { "createdAt": "2020-06-28T18:02:27Z", "name": "clojurescript" }, { "createdAt": "2020-06-28T18:06:55Z", "name": "io" }, { "createdAt": "2020-06-28T18:09:36Z", "name": "forth" }, { "createdAt": "2020-06-28T18:13:09Z", "name": "05ab1e" }, { "createdAt": "2020-07-26T19:26:47Z", "name": "nix" }, { "createdAt": "2020-11-18T17:45:14Z", "name": "solidity" }, { "createdAt": "2021-01-05T16:36:23Z", "name": "zig" }, { "createdAt": "2021-02-09T14:58:12Z", "name": "red" }, { "createdAt": "2021-02-16T15:17:42Z", "name": "z3" }, { "createdAt": "2021-05-04T15:06:46Z", "name": "wren" }, { "createdAt": "2021-09-16T15:32:10Z", "name": "qsharp" }, { "createdAt": "2021-09-30T15:13:58Z", "name": "abap" }, { "createdAt": "2021-10-14T11:34:03Z", "name": "gnucobol" }, { "createdAt": "2021-12-09T19:27:26Z", "name": "vlang" }, { "createdAt": "2022-01-26T23:23:39Z", "name": "wasm" }, { "createdAt": "2022-02-12T19:40:53Z", "name": "free-pascal" }, { "createdAt": "2022-05-05T06:25:50Z", "name": "unison" }, { "createdAt": "2022-06-03T10:46:36Z", "name": "awk" }, { "createdAt": "2022-06-29T05:44:11Z", "name": "8th" }, { "createdAt": "2022-07-11T19:16:49Z", "name": "cobol" }, { "createdAt": "2022-08-25T09:51:21Z", "name": "jq" }, { "createdAt": "2022-09-22T19:09:59Z", "name": "bqn" }, { "createdAt": "2022-10-05T08:05:09Z", "name": "chapel" }, { "createdAt": "2022-10-20T10:08:50Z", "name": "openeuphoria" } ] ```
ee7 commented 1 year ago

This script would be used by any contributor/maintainer who is developing on a Windows machine, right?

Sort of. I guess it depends on whether they prefer PowerShell?

I think that a lot of developers on Windows use bash, and/or WSL. And I imagine that more people can read sh/bash than PowerShell, even developers on Windows.

But PowerShell is installed by default on every Windows OS from Windows 7 onwards, and bash isn't, so I do think there's value in providing the PowerShell script. It just takes more effort to maintain two scripts that do the same thing, and I don't know how many potential configlet users only have PowerShell installed.

But I left Windows a long time ago, so I'll defer to @ErikSchierboom here.

ee7 commented 1 year ago

Also, about half of the fetch-configlet.ps1 scripts aren't up to date, and the csharp track has a unique one with non-upstreamed changes:

$ find . -name 'fetch-configlet.ps1' -exec md5sum {} + | sort
83fd8dfbde28fab5d05e54ceec9aca5a  ./abap/bin/fetch-configlet.ps1
83fd8dfbde28fab5d05e54ceec9aca5a  ./qsharp/bin/fetch-configlet.ps1
83fd8dfbde28fab5d05e54ceec9aca5a  ./red/bin/fetch-configlet.ps1
83fd8dfbde28fab5d05e54ceec9aca5a  ./wren/bin/fetch-configlet.ps1
83fd8dfbde28fab5d05e54ceec9aca5a  ./z3/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./8th/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./awk/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./bqn/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./chapel/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./cobol/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./dart/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./free-pascal/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./gnucobol/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./jq/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./openeuphoria/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./unison/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./vlang/bin/fetch-configlet.ps1
88ebec802498c789810c9b7f6e74413e  ./wasm/bin/fetch-configlet.ps1
9edfc3e55870eb9476ba178524d1ec84  ./cpp/bin/fetch-configlet.ps1
9edfc3e55870eb9476ba178524d1ec84  ./fsharp/bin/fetch-configlet.ps1
9edfc3e55870eb9476ba178524d1ec84  ./javascript/bin/fetch-configlet.ps1
9edfc3e55870eb9476ba178524d1ec84  ./solidity/bin/fetch-configlet.ps1
9edfc3e55870eb9476ba178524d1ec84  ./typescript/bin/fetch-configlet.ps1
9edfc3e55870eb9476ba178524d1ec84  ./zig/bin/fetch-configlet.ps1
c1fc9dc384a9969a398212a71a752e3c  ./csharp/bin/fetch-configlet.ps1
ErikSchierboom commented 1 year ago

I think that a lot of developers on Windows use bash, and/or WSL. And I imagine that more people can read sh/bash than PowerShell, even developers on Windows.

In my experience, this is not true. I've worked in a lot of Windows environments and PS was used for more than sh/bash.

But PowerShell is installed by default on every Windows OS from Windows 7 onwards, and bash isn't, so I do think there's value in providing the PowerShell script. It just takes more effort to maintain two scripts that do the same thing, and I don't know how many potential configlet users only have PowerShell installed.

There is, but it isn't that these scripts change that much, so I think there's value in keeping them.

ee7 commented 1 year ago

Well, I think it would make sense to have them sync too, but only for the tracks interested in them. We could maybe use the org-wide-files-config.toml file that we also used to enable or disable format checking

so I think there's value in keeping them.

As long as I don't have to maintain it :)

So, let's add fetch-configlet.ps1 to org-wide-files, but make it opt-in - by default only syncing to tracks that already have the file present? org-wide-files can do that, right?

ErikSchierboom commented 1 year ago

org-wide-files can do that, right?

Not yet no. And I don't know Julia: https://github.com/exercism/org-wide-files/blob/main/scripts/apply-track-config.jl 😱

kytrinyx commented 1 year ago

Is there a mechanism for tracks to configure which (optional) files they want? If so I think it's fine to make the .ps1 file optional and only sync it if the track adds it to the configuration.

ErikSchierboom commented 1 year ago

Is there a mechanism for tracks to configure which (optional) files they want? If so I think it's fine to make the .ps1 file optional and only sync it if the track adds it to the configuration.

There isn't, we'd have to build it ourselves.

kytrinyx commented 1 year ago

I think my suggestion right now is to get the basic fetch-configlet script included now, and leave the .ps1 script off. Let's just leave people to get ps1 updated if they care about it (for now).

ErikSchierboom commented 1 year ago

Sounds like a plan

ee7 commented 1 year ago

Closed by: https://github.com/exercism/org-wide-files/pull/267

Let's track the idea to add fetch-configlet.ps1 in a separate issue: #280