Issue 20: Exercise Names + A Refactor to Bring Into Spec Compliance

[BethanyG]

As stated on the 🥫 , a refactor to bring the analyzer "up to spec" 🌈 ✨ :

• Altered (per Modify Analyzer to Pull Exercise Names from <indir>/.meta/config.json where Possible #20 ) to read in/out/test from an exericses config.json
• Altered to remove approve/disprove/refer to mentor top-level statuses
• Changed Status to Summary and added summary comments categories
• Added a dataclass to represent a Comment, with CommentTypes, params and comments text
• Added an Enum to represent CommentTypes for flagging each comment given.
• Mapped the feedback and message types given back from pylint to the ComentTypes defined for exercism.
• Pulled Pylint specific code into its own file, since it was generic.
• Did some cleanup of names and imports
• Changed the output-format to conform to the spec listed here: https://github.com/exercism/docs/blob/main/building/tooling/analyzers/interface.md#output-format
• Cleaned up tests to remove checks for the approve/disapprove/refer to mentor statuses
• Further refactored tests to account for dataclass, Enum, and output_file changes
• Pulled summary comment code out of two-fer/analyzer, made it generic and moved it to a @classmethod of Analysis

Also added CI-related files.

More work is needed to clean up code and better integrate PyLint, but this is a functional first pass. 🎉

[iHiD]

I've made a few little comments.

One thing I'm not quite able to work out from a cursory glance is what happens if there are a mixture of comment types for one solution? If for example there is a actionable and a informative comment, what does the top-level message do?

[BethanyG]

@iHiD -- Thank you for the comments (and the spelling patrol!!)! 😄

One thing I'm not quite able to work out from a cursory glance is what happens if there are a mixture of comment types for one solution? If for example there is a actionable and a informative comment, what does the top-level message do?

The "highest priority" comment takes precedence in that circumstance, and a a DIRECT ("There are a few changes we suggest that can bring your solution closer to ideal." -- which maps to ACTIONABLE comments) summary message would be returned. The logic is in a @classmethod on Analysis -- starting at line 78 in analysis.py.

Frankly, this was me not (quite) wanting to throw away the old APPROVE/DISAPPROVE organization - so I turned it into a summary. Not sure I am happy with that, and I might get rid of it as I proceed down the road to integrating pylint better. Open to thoughts/suggestions/ideas on that front.

I did add a summary status of GENERIC, for when there were no exercism specific comments, and only pylint returned anything. Haven't wired that up yet. The comment Enums are in comment.py

One thought is to write a "generic" analyzer (with that GENERIC summary message) that ran a student's solution through pylint only, so that we could at minimum have a pylinting for every exercise. Then double back and write/customize individual analyzers for exercises as we get to them. Open to suggestions on that as well. Pylint uses a pretty nice lib for AST wallking (astriod), that I'm eager to get my hands on. 😉

[iHiD]

Thank you for the comments (and the spelling patrol!!)! 😄

Of course 🙂

Open to thoughts/suggestions/ideas on that front.

I've not done this yet for the Ruby one, but my (medium-term) plan is to add summary comments where I feel they add some explicit extra value. The website itself should handle informing the student what to do in general (ie "please sort this before proceeding") so the aim of this field was for the analyzer to be able to give extra information where that wasn't enough. That said, in the short-term I'll probably do exactly the sort of thing you've done here. One to explore further down the line for sure 🙂

[BethanyG]

@cmccandless @iHiD @SleeplessByte -- are there additional points I need to address here for approval? Just checking....

[iHiD]

(Not from me, but you don't need my code-level-approval anyway as this doesn't touch anything I'm a codeowner of. I'll approve so you can merge when you're ready 🙂)

[cmccandless]

I think dev-requirements.txt.py is an accident. Once that's removed, I'm good with this, so I'll approve now.

[iHiD]

Whoop 🎉