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

Add analyzer feedback for Annalyns Infiltration #2700

Merged
merged 6 commits into from
Feb 2, 2024

Conversation

kahgoh
Copy link
Member

@kahgoh kahgoh commented Jan 31, 2024

pull request

closes #2669


Reviewer Resources:

Track Policies

Copy link
Contributor

@manumafe98 manumafe98 left a comment

Choose a reason for hiding this comment

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

Checking on the Javascript analyzer approach I would say that we could be more explicit on some actionables, I like the topics already created, but maybe we could add a couple more, I also would like your opinion on this one @sanderploegsma

The if statement in the method seem to be treated as essential as well, check this

@sanderploegsma
Copy link
Contributor

I think that given the scope of the concept and the exercise, we should steer students towards a solution that only uses boolean operators (&&, || and !). If a solution uses if-statements, ternary expressions or comparison operators and boolean literals, I think the analyzer comments should be of type essential.

In terms of additional comments, I can think of only one thing, and that is unnecessary parentheses in the final method. If there is a consistent way to check this, could be an informative comment if the rest of the solution doesn't receive any feedback.

@kahgoh
Copy link
Member Author

kahgoh commented Feb 1, 2024

Making them essential sounds good to me ;)

Thanks for pointing me to the Javascript analyzer @manumafe98. I've tried taking some inspiration from there for the wording and added something about the unnecessary parentheses.

@manumafe98
Copy link
Contributor

Thanks for pointing me to the Javascript analyzer @manumafe98. I've tried taking some inspiration from there for the wording and added something about the unnecessary parentheses.

Np! this is already looking great!

@manumafe98 manumafe98 merged commit de59086 into exercism:main Feb 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

annalyns-infiltration: describe Analyzer feedback in .meta/design.md
3 participants