Proposal for tagging mechanism

[jiegillet]

Dear @mpizenberg, @ceddlyburge and @ErikSchierboom,

Following our (fun and productive) discussion about tags, I came up with a prototype for the tagging mechanism, using elm-review's data extractors, which seems like a perfect fit, I have been able to add some tags very quickly.

Some comments:

• All of the tagging logic is in src/Tags.elm. If you are not familiar with elm-review, you can ignore the boilerplate, the core function is matchExpression and is easy to understand.
• For now, I have added random tags, some because they were easy, and because they were a bit more subtle (LetExpression, multiline strings).
• There is one rule commonTagsRule that always returns the same tags commonTags for any project.
• For more complex traversals, for things like recursion, I would recommend adding a separate rule.
• I don't know if we want to have more structure around the rules, for now one rule for each type of elm-syntax type (project, module, expressions, function declarations...) seems the most natural fit.
• I have been testing every case so far, and I have found bugs thanks to it
• I haven't added smoke tests for now, but it will be simple, we can do it once we have more tags in, they will be easy to add.- @ErikSchierboom I'm not sure if it can go live or not in this incomplete state. Is there a mechanism for going live? The tags.json files will be created from this PR on anyway.

I'd be happy to hear feedback about any of it.

[mpizenberg]

This sounds like a solid starting point for all things that do not need more-than-one-node analysis! I was wondering if uses:prefix-operator could be a tag? seems like something we might want to know (and forbid ahah, at least for operators that are not symmetric).

[jiegillet]

An alternative would be to don't yet output the tags.json file and put it behind a feature flag. Ideally we would not have to re-run the analyzer on all solutions multiple times.

@ErikSchierboom How would a feature flag work?

This sounds like a solid starting point for all things that do not need more-than-one-node analysis! I was wondering if uses:prefix-operator could be a tag? seems like something we might want to know (and forbid ahah, at least for operators that are not symmetric).

I actually added that very tag on the spreadsheet, but I forgot to include it in the code :)

[jiegillet]

Thank you for the feedback so far. I made some changes and I added a bunch of tags, but it's still WIP. Could I ask for your help on some of the decisions? On the spreadsheet I marked some questions in orange wherever I couldn't make a clear decision, please add some decisive comments on there so we can make progress :)

[ErikSchierboom]

Ideally do a canary release, just using it for a small subset of solutions (maybe just solutions done by the maintainters or something like that). Not sure how easy / possible this is from an exercism point of view though.

We don't yet have anything like this and I wouldnt' really want to start adding exceptions, sorry.

Maybe have some monitoring / logging for a while at the start, so we can keep an eye on it. Not sure what this would entail, maybe it could use the AI tags as well and flag if anything is very different or something like that. Not totally sure this is worth it, but would provide more confidence.

This sounds somewhat complicated. Personally, having a nice set of smoke tests should give us enough confidence that this is working.

[ErikSchierboom]

@ErikSchierboom How would a feature flag work?

Well, the thing I would like to enable is for you all to continue working on adding more tags whilst still having CI verify the tags output. So maybe a docker build arg that is off by default?

[jiegillet]

I know I won't have a lot of free time in the next few weeks, so I'll stop working on this PR further and submit it as it is, it's already very big anyway. Some notes:

• I have added a feature flag, bin/run.sh slug input output --tags. If the 4th argument is missing or anything other than --tags then the tags.json won't be written. run-in-docker.sh doesn't have it and run-tests-in-docker.sh has it.
• I added smoke tests for tags for the existing test data we have, but it would be nice to add more specific ones later.
• There is still a number of tags not implemented, that can be done later in smaller chunks
• If you want to add a tag or something, add it on the spreadsheet
• I will open an issue with a roadmap for deploying the tags feature
• @ErikSchierboom I have asked for a review from you, obviously you don't need to check the Elm code, but please check the scripts