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

Feature: Merge annotations (if they are more than X) before submitting #41

Closed
Luro02 opened this issue Dec 21, 2023 · 2 comments · Fixed by #117
Closed

Feature: Merge annotations (if they are more than X) before submitting #41

Luro02 opened this issue Dec 21, 2023 · 2 comments · Fixed by #117
Assignees

Comments

@Luro02
Copy link
Collaborator

Luro02 commented Dec 21, 2023

Is your feature request related to a problem?

The autograder is very thorough when grading, so it produces a lot of annotations.
There can be 100+ annotations for a submission, which is a challenge for a student to review.
It becomes more likely that the student does not see important feedback or does not look through them all, which is unfortunate.

There are some lints that make sense to group like

  • use format strings ("%d%s".formatted(number, string)),
  • non-descriptive constant names
  • unused code

but there are other lints like common reimplementation or use different visibility that provide individual suggestions for specific code and therefore would loose a lot of value by merging them.

A potential solution should therefore conditionally group annotations

Describe the solution you'd like

The simplest solution I can think of, is to do the following:

The autograder emits for each annotation an id (could be a number).

Before submitting, the grading tool will

  • count all annotations with the same id
  • if there are more than for example 5 with the same id
  • the first 4 are kept as is and the remaining are merged into one annotation
  • the annotation could look like this: "The identifier 'intField4' should not contain its type in the name. Other problems in L7, L8, L9, L10, L11."

Describe alternatives you've considered

The autograder merges lints for now, but this results in a few problems:

  • false-positives made by the autograder are harder to find, because they might be hidden in a message and not marked in the code.
  • one might think that some pieces have been forgotten by the autograder (because they are not marked)

Additional context

This issue has been created in kit-sdq/eclipse-artemis#326, but should be implemented in this project.

@Luro02
Copy link
Collaborator Author

Luro02 commented Jul 17, 2024

Any ideas on how this should look like? @dfuchss @Feuermagier

If not, then I would go with the Autograder implementation that I already wrote. Looks like this:

[Custom Kommentare:Individueller Kommentar] '"Error: author %s not found.".formatted(author)' ist schöner zu lesen. Weitere Probleme in BibliographyManager:L20, Publication:(L21, L22, L23, L24, L25). (0P)

@Luro02 Luro02 self-assigned this Jul 17, 2024
@dfuchss
Copy link
Member

dfuchss commented Jul 17, 2024

I guess something like that would be good :)

Luro02 added a commit to Luro02/artemis4j that referenced this issue Sep 24, 2024
Luro02 added a commit that referenced this issue Sep 24, 2024
Luro02 added a commit that referenced this issue Sep 24, 2024
@github-project-automation github-project-automation bot moved this from Triage to Done in Artemis & Programming Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants