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

Kotlin String Implementaiton #759

Merged
merged 7 commits into from
Oct 27, 2024

Conversation

apurvabanka
Copy link
Contributor

@apurvabanka apurvabanka commented Oct 23, 2024

What GitHub issue does this PR apply to?

Resolves #741

What changed and why?

Implemented changes for Kotlin String.

image

(If editing website code) Please add screenshots

Checklist

  • I claimed any associated issue(s) and they are not someone else's
  • I have looked at documentation to ensure I made any revisions correctly
  • I tested my changes locally to ensure they work
  • For language files, I have validated the edited files are valid JSON and data shows up correctly
  • For website code edits, I have added or edited any appropriate unit tests for my changes

Any additional comments or things to be aware of while reviewing?

@geekygirlsarah
Copy link
Member

geekygirlsarah commented Oct 23, 2024

From the validating checks:

Error:  `kotlin/1.5/strings.json`, ID: `count_occurrences_of_substring` has not_implemented (underscore) when it should use not-implemented (hyphen)
Error:  `kotlin/1.5/strings.json`, ID: `convert_to_title_case` has not_implemented (underscore) when it should use not-implemented (hyphen)
Error:  `kotlin/1.5/strings.json`, ID: `parameter_format_in_order` has not_implemented (underscore) when it should use not-implemented (hyphen)
3 errors found.

I think I also saw a "not-implemented": "true" (with quotes around true) that will need to get fixed. But past that it looks good! Fix those things and I'm happy to merge it in!

(The docker check is an issue related to a library it uses, and isn't a fault of this PR so I'm ignoring it)

@apurvabanka
Copy link
Contributor Author

From the validating checks:

Error:  `kotlin/1.5/strings.json`, ID: `count_occurrences_of_substring` has not_implemented (underscore) when it should use not-implemented (hyphen)
Error:  `kotlin/1.5/strings.json`, ID: `convert_to_title_case` has not_implemented (underscore) when it should use not-implemented (hyphen)
Error:  `kotlin/1.5/strings.json`, ID: `parameter_format_in_order` has not_implemented (underscore) when it should use not-implemented (hyphen)
3 errors found.

I think I also saw a "not-implemented": "true" (with quotes around true) that will need to get fixed. But past that it looks good! Fix those things and I'm happy to merge it in!

(The docker check is an issue related to a library it uses, and isn't a fault of this PR so I'm ignoring it)

My bad !!!! Fixed it.

@geekygirlsarah
Copy link
Member

From the validating checks:

Error:  `kotlin/1.5/strings.json`, ID: `count_occurrences_of_substring` has not_implemented (underscore) when it should use not-implemented (hyphen)
Error:  `kotlin/1.5/strings.json`, ID: `convert_to_title_case` has not_implemented (underscore) when it should use not-implemented (hyphen)
Error:  `kotlin/1.5/strings.json`, ID: `parameter_format_in_order` has not_implemented (underscore) when it should use not-implemented (hyphen)
3 errors found.

Those still need to be fixed, they failed the validating check again.

@apurvabanka
Copy link
Contributor Author

@geekygirlsarah Are there any instructions on how to run the tests?

@geekygirlsarah
Copy link
Member

I swore we had a docs page on it, but I'm not seeing it. Either I'm tired or it doesn't exist! 😆

you can run:
python manage.py validatemetainfofile
to validate meta_info.json and adding a new language/concept, or
python manage.py validatelanginfofiles
to validate individual files (like what you're doing)

Ideally run both to get a full check on it.

@geekygirlsarah
Copy link
Member

codethesaurus/docs#42

Clearly I thought this through a year ago! 😆

@apurvabanka
Copy link
Contributor Author

apurvabanka commented Oct 25, 2024

@geekygirlsarah Added some testing docs as well for the contribution page. All test cases are passing now.

Copy link
Member

@geekygirlsarah geekygirlsarah left a comment

Choose a reason for hiding this comment

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

I see the tests pass, so that helps! I'll merge this in with the README adjustments!

CONTRIBUTING.md Outdated Show resolved Hide resolved
@geekygirlsarah
Copy link
Member

Ignoring the Docker build issue, this is good to go! I'll merge it in.

Happy Hacktoberfest! This counts towards that if you're participating!

@geekygirlsarah geekygirlsarah merged commit b7ae270 into codethesaurus:main Oct 27, 2024
4 of 5 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.

[Kotlin] Add strings
2 participants