Add message to PG critic output if no issues are found. #2876
Conversation
| </div> | ||
| % last; |
There was a problem hiding this comment.
@drgrice1 I just realized these aren't needed, as the following two conditionals will fail and the only output after this is the trailing div. Any preference on if I should remove lines 9 and 10 and have two more if checks?
There was a problem hiding this comment.
Yeah, please remove the </div> and % last; lines. I really don't like unbalanced code in the templates, and would prefer to avoid it when it can be done. You added a similar thing to the file templates/ContentGenerator/ProblemSet/auxiliary_tools.html.ep in pull request #2869. I am allowing that for now, but I don't like it. Here it isn't needed.
There was a problem hiding this comment.
The problem I have with this sort of thing is that it can make it difficult to check HTML validity by analyzing the code. In this case there is a <div> that starts, and then there are two corresponding </div> endings. The old theme templates had a ton of this, and it was a nightmare to sort out.
somiaj
force-pushed
the
pg-critic-msg-if-no-issues
branch
from
23b27d9 to
91e299e
Compare
3 participants
I found it confusing when no message was given when there were no pg critic violations.
This adds a message that no violations were found. I couldn't think of much to say, so this is simple now, but was thinking maybe it could contain other information to help problem authors even though their problem code doesn't trigger any pg critic violations.