PG::Critic add policy for weightedGrader.pl functions.

[somiaj]

Add a PG::Critic policy to catch use of the deprecated weightedGrader.pl function calls.

[drgrice1]

Technically the weightedGrader.pl macro is not yet officially deprecated. It has not been moved to the macros/deprecated directory, which is the official mark of deprecation at this point.

I really don't think that it is a good idea to add a specific macro deprecation rule for a macro that is not deprecated. Particularly when there is already a general rule for actually deprecated macros.

[drgrice1]

I don't think that it is a good idea to include this comment.

First, I would not say that using the "Edit Tags" button in webwork2 is the preferred way of editing tags. There is certainly nothing wrong with directly editing them in a file. There are also other ways of editing tags that in the future might even become more preferable. This is specific to webwork2, and the standalone renderer also has a tag editor.

Second, the "Edit Tags" button is only shown for admin users in a course. So for other instructors that are creating and editing problems that isn't even an option.

Finally, I am not even sure this rule is a good idea. I added it because it was in the rules discussed at the Vegas workshop. However, there are problems with this. One issue is the fact that the defined tags which are from webwork-open-problem-library/OpenProblemLibrary/Taxonomy2 may not work for all problems. Another issue is that the tags are not even complete to begin with. For example, the DBSubject of "Graph theory" has a DBChapter of "Terminology", but there is no DBSection to go with that. The same for the DBChapter of "Coloring". There are other cases like this as well.

So it is likely that we should disable this rule until we get the tags in proper order to begin with.

Once the tags are in order and we think this rule is a good idea, then this rule should probably even go further and ensure that the tags are not only set, but have valid values. Although the issue with that is that the tags are defined in a file in another repository.

[drgrice1]

This is not true. If this comment is going to be added then it needs to be more specific. You seem to be assuming that this is only used through the webwork2 interface. This can also be used with the bin/pg-critic.pl script (or in theory other implementations). The no critic annotations are shown by default, but can be disabled. If the -s flag is passed to the pg-critic.pl script. then the no critic annotation is ignored and all policies are forced to apply.

[drgrice1]

I guess the new rule is okay, since we do have the comment in the POD that the macro shouldn't be used, and the rule is different than the general macro deprecation rule in that it is checking method calls.

[somiaj]

My thought was also to help explain how to fix the issues, and just stating the macro is deprecated might not help an author know how to weight answers. I modeled this off of ProhibitDeprecatedMultipleChoice.pm which mentions what macros to use and also gave sample problems to help authors switch.

I am okay with removing PG::RequireMetadata, and agree that if we have it, we should check the values. I was just trying to add some useful information vs that metadata was missing, such as pointing out the tag editor and a way to disable those messages if you didn't want to deal with meta data for local problems. I thought others who might use the tool to help improve their problems would like such information.

[somiaj]

I just dropped my second commit with modifying the PG::Metadata POD. I wasn't exactly sure what others would think, but overall I think the descriptions in the POD can be expanded to help authors who may not be that familiar with PG know how to fix the issue.

[drgrice1]

I am not opposed to the comment regarding the no critic annotation. I was just saying that if that is added it needs to me more precise. Just add a note that the no critic annotation is ignored when PG critic is in "strict" mode.

[somiaj]

I mostly removed it due to I would rather just disable it per your previous comment, than tell people to ignore it. So I think it should also be removed/disabled until the tag situation is better worked out. I find I'm just ignoring/disabling it in my use and thought others would want to do the same.

But to me, if many are just going to disable that policy, might as well just not have it, or at the least I don't want to couple it with the weightedGrader.pl policy for this PR at this time, and it should probably be discussed further.