Skip to content

Conversation

@manumafe98
Copy link
Contributor

@manumafe98 manumafe98 commented Feb 19, 2024

closes #107

To do:

@manumafe98 manumafe98 added the x:size/medium Medium amount of work label Feb 19, 2024
@manumafe98 manumafe98 self-assigned this Feb 19, 2024
@manumafe98 manumafe98 requested a review from a team as a code owner February 19, 2024 20:15
Copy link
Contributor

@sanderploegsma sanderploegsma left a comment

Choose a reason for hiding this comment

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

Overall it's looking very neat, nice work so far!

manumafe98 and others added 4 commits February 21, 2024 10:25
Co-authored-by: Sander Ploegsma <sanderploegsma@gmail.com>
Renaming comment of string format to prefer_string_concatenation
Adding another helper method callsMethod
Making comment about substrings general
Adding another tests case for when substring is not being used in both methods
Making the analyzer to return only the hardcode and substring method if those trigger, so we do not overload the student of comments
@manumafe98
Copy link
Contributor Author

@sanderploegsma I've found that for hardcoded tests it was being used this general comment I should use this one? Or the one I created is ok?

@sanderploegsma
Copy link
Contributor

@sanderploegsma I've found that for hardcoded tests it was being used this general comment I should use this one? Or the one I created is ok?

Great job for noticing, I forgot about that. If there is a general comment already, please use that instead of adding a new one! The only exception would be if you want the comment copy to contain more information that perhaps is exercise-specific, but I don't think that's the case here.

Using general comment for hardcoded tests
Updating prefer string concatenation comment to be a general one
@manumafe98
Copy link
Contributor Author

@sanderploegsma I was thinking if we should also make this one a general comment ReuseCode if we are being vague with the lasagna one we can apply it to this one as well

@sanderploegsma
Copy link
Contributor

@sanderploegsma I was thinking if we should also make this one a general comment ReuseCode if we are being vague with the lasagna one we can apply it to this one as well

If you have a good idea for it then by all means! I suggest taking a look at the website copy first to see how it all works out.

@manumafe98
Copy link
Contributor Author

If you have a good idea for it then by all means! I suggest taking a look at the website copy first to see how it all works out.

Great! mostly because I was thinking on future analyzers that could also use the reusecode comment, so for me it makes more sense to make it a general one!

Copy link
Contributor

@sanderploegsma sanderploegsma left a comment

Choose a reason for hiding this comment

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

Awesome work! :shipit:

@manumafe98 manumafe98 merged commit c392d59 into exercism:main Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

x:size/medium Medium amount of work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

log-levels: implement analyzer

3 participants