exercism / v3

The work-in-progress project for developing v3 tracks
https://v3.exercism.io
Other
170 stars 162 forks source link

Convert editor key in .meta/config.json files #3087

Closed ErikSchierboom closed 3 years ago

ErikSchierboom commented 3 years ago

We're renaming the "editor" field in the .meta/config.json file to "files", as their usage isn't just restricted to the editor (we can use them for the CLI too).

See https://github.com/exercism/v3-docs/pull/25/files

I've also taken the liberty of adding the "files" field when it wasn't yet specified in an exercise.

mpizenberg commented 3 years ago

@SaschaMann the key is welcomed for the elm track because of the naming issue discussed in previous PR. I suppose it could be optional and only used in tracks where exemplar.... is not a good file name.

iHiD commented 3 years ago

What's the motivation behind introducing it?

Traversing a filesystem tree looking for any file matching a regexp that's in any number of possible nested directories isn't something we want to be doing every time. It's both expensive in terms of time and resources, and also in terms of cost in accessing unnecessary files across networked filesystems. It's also more error-prone.

So, although I understand that it's annoying having that duplication, it's something we're going to push on with, along with the solution files and test files in this config.

To help reduce any unnecessary work for tracks, it feels like it would be relatively trivial to make a small tool that can automatically generate a config.json on an as-needed basis (e.g. when exercises are created) based on a track's own config. For example a track provides "solution", "test" and "examplar" regexps and then the file is created with populated from those. Maybe this is something that we can do moving forward? However for now, I want to prioritise getting v3 over the line.

That aside, if you think this key is needed, the existing CI check for the existence of .meta/config files should be extended to check these files, too.

Agreed. Confliget should be testing all keys in this file. I know @ee7 is close to having the first pass of this ready for you all to work with :)

SaschaMann commented 3 years ago

Agreed. Confliget should be testing all keys in this file. I know @ee7 is close to having the first pass of this ready for you all to work with :)

There is already an existing check in place until configlet is ready, which should be extended until then.

SaschaMann commented 3 years ago

Traversing a filesystem tree looking for any file matching a regexp that's in any number of possible nested directories isn't something we want to be doing every time. It's both expensive in terms of time and resources, and also in terms of cost in accessing unnecessary files across networked filesystems. It's also more error-prone.

It doesn't require a regex or any traversal, a track-level config.json entry will do.

ErikSchierboom commented 3 years ago

There is already an existing check in place until configlet is ready, which should be extended until then.

You mean the v3 workflow?

It doesn't require a regex or any traversal, a track-level config.json entry will do.

Well yes, for many tracks it could be defined in one place. And I imagine us supporting that in the future.

SaschaMann commented 3 years ago

You mean the v3 workflow?

Yes

iHiD commented 3 years ago

It doesn't require a regex or any traversal, a track-level config.json entry will do.

Yep. Makes sense. We'll PR a track-wide key next week to tracks, and they can choose to accept or remove the PR. But we're not going to have that ready today, so it'll have to be a follow-on PR.

ErikSchierboom commented 3 years ago

There is already an existing check in place until configlet is ready, which should be extended until then.

We can relatively quickly build this into configlet.

SaschaMann commented 3 years ago

(I have no idea why that removed the review requests, I didn't click on those tracks)

ErikSchierboom commented 3 years ago

Thanks!