Skip to content
New issue

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

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

Already on GitHub? # to your account

Linting rule to check formatting of translation strings #11681

Merged
merged 2 commits into from
Jan 2, 2024

Conversation

KshitijThareja
Copy link
Contributor

Summary

This PR introduces a new linting rule to check if the translation strings defined in $trs and createTranslator functions are properly formatted, i.e, they have a specific shape and their properties are of a specific datatype.

Sample error

errorlog

References

Closes #11301

Reviewer guidance

To test these changes:

  1. Modify any translation string defined in a Vue or JS file so that it has an improper format.
  2. Run the linting command: yarn run lint-frontend.
  3. You'll see the particular translation string's name logged in the terminal.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added DEV: tools Internal tooling for development SIZE: medium labels Dec 31, 2023
@KshitijThareja
Copy link
Contributor Author

Hi @AlexVelezLl 👋🏻
I've covered most of the requirements for this issue. I couldn't figure out how to add a check to ensure that the message property is of type string as there are some message keys containing variables, which gets classified as 'undefined' when checked for their type. Other than that, everything else works as expected.
Also, please mention if you have any other suggestions that you'd like to be incorporated in the code. I'll be happy to work on them :)

@rtibbles
Copy link
Member

rtibbles commented Jan 2, 2024

This looks great, thanks @KshitijThareja and it looks like you have discovered an existing issue here! https://github.com/learningequality/kolibri/blob/release-v0.16.x/kolibri/plugins/learn/assets/src/views/ExploreLibrariesPage/index.vue#L176

Could you correct continue to context here, so that the linting passes and we can merge this?

@KshitijThareja
Copy link
Contributor Author

Sure @rtibbles. I'll make the changes 👍🏻

@github-actions github-actions bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend labels Jan 2, 2024
@rtibbles rtibbles merged commit 8f00a2e into learningequality:release-v0.16.x Jan 2, 2024
33 checks passed
@rtibbles
Copy link
Member

rtibbles commented Jan 2, 2024

Thank you @KshitijThareja great work!

@KshitijThareja
Copy link
Contributor Author

Welcome @rtibbles 😄
It was personally a great experience working on this issue. Looking forward to contribute further 👍🏻

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend DEV: tools Internal tooling for development SIZE: medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linting does not pick up on badly formatted string objects
2 participants