Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: hints for failing checks #156

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

dyga01
Copy link
Collaborator

@dyga01 dyga01 commented Nov 5, 2024

Hint Feature

Description

This feature allows instructors to add hints that will be displayed anytime a check fails. The hints can be added under options in the gatorgrade.yml file. This will be very beneficial because it provides instructors with an opportunity to provide additional information for more difficult checks.

Linked Issues

closes: #155

Type of Change

  • Feature
  • Bug fix
  • Documentation

Contributors

Images

This is an example of the feature on a hint for a very simple check.

gatorgrade --config config/gatorgrade.yml on normal repository

Screenshot 2024-11-05 at 12 06 08 PM

gatorgrade --config config/gatorgrade.yml on repository that also includes execexam

Screenshot 2024-11-08 at 10 58 22 AM

@dyga01 dyga01 added the enhancement New feature or request label Nov 5, 2024
Copy link
Collaborator

@rebekahrudd rebekahrudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For others running this I manually added a hint in my gatorgrade.yml file. Also from what I found, hints can only be added to the "options" lists and not just any command in the gatorgrade.yml. I tried adding a hint to another command that failed and did not see the hint output.

Can commands without the "options" list have hints? It might be worth adding it as it's own category rather than as part of the "options" list in the gatorgrade.yml file. This way professors can add a hint to any element of the gatorgrade.yml file.

Also, when both the run this command and the hint prints there is a space between the command that ran and the hint. This would be worth addressing. It makes it confusing to read. I hard coded a new line character in the run this command. A possible fix might be adding the hint before the run this command or figuring out a way to only add the newline character if there is no hint after it.

The changes I made to the gatorgrade.yml file:
gatorgrade yml file changes

Here is the output I see in my terminal when running this:
terminal output w the hint

To fix:

  • move hint out of the options list in the gatorgrade.yml (but this also could be me just not quite understanding the hint feature)
  • remove the newline from run this command when hint and run this command are both print
  • consider: changing the color scheme so that hint is more easily differentiable from run this command

@dyga01
Copy link
Collaborator Author

dyga01 commented Nov 5, 2024

@rebekahrudd Thanks for thoroughly reviewing this feature! I think our class should talk about everything you mentioned during my demo.

@gkapfham
Copy link
Collaborator

Hi @dyga01 there are changes requested for this PR, can you please resolve them and then report back as to your status?

@CalebKendra
Copy link
Collaborator

Hi @dyga01, do you have any update for this pr?

@dyga01 dyga01 requested a review from rebekahrudd November 22, 2024 00:16
@dyga01
Copy link
Collaborator Author

dyga01 commented Nov 22, 2024

Hi @dyga01, do you have any update for this pr?

Hi @CalebKendra, this PR has been updated to work with shell checks and also now works with Rebekah's "run this command" feature. I think this PR is ready to be reviewed. Thanks!

Copy link
Collaborator

@rebekahrudd rebekahrudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked on Linux!

Here is the output of adding a hint to a ShellCheck command that calls gatorgrader:
ss hint GGer

Here is the output of adding a hint to a GatorGradeCheck:
ss hint gg

To test this PR install gatorgrade using the following command: pipx install --force git+https://github.com/dyga01/gatorgrade.git@hint_feature

Copy link
Collaborator

@PCain02 PCain02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works on Windows 10!
image

I used the Windows bug as an example for fun. The "MatchFileFragment" is something @suppo01 and @rebekahrudd are working on now but I thought it would be interesting if I used the hint feature with it. Good job overall, I think this format will be useful for future features. One question, I saw a test case was deleted and nothing replaced it why was that?

Copy link
Collaborator

@PCain02 PCain02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I meant to wait to approve until I got a response about the test case.

@dyga01
Copy link
Collaborator Author

dyga01 commented Dec 6, 2024

Sorry I meant to wait to approve until I got a response about the test case.

@PCain02 This test case has been added back in. Thanks!

Copy link
Collaborator

@AlishChhetri AlishChhetri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Collaborator

@rebekahrudd rebekahrudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me on my Linux computer!

Copy link
Collaborator

@hemanialaparthi hemanialaparthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this on mac and works as intended! LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: hints on failed checks
8 participants