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 prefer-const linting rule to kolibri-tools #10130

Merged
merged 3 commits into from
Feb 23, 2023

Conversation

Akila-I
Copy link
Contributor

@Akila-I Akila-I commented Feb 18, 2023

Summary

  • New ESLint rule (prefer-const) added
  • Errors prompted by prefer-const lint rule are fixed
  • performed npm run lint-frontend. No errors found.
  • PR does not affect the UI

References

Reviewer guidance

  • npm run lint-frontend to check ESLint rules compliance.

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 APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) SIZE: small labels Feb 18, 2023
@Akila-I Akila-I marked this pull request as ready for review February 18, 2023 16:34
@MisRob MisRob requested review from rtibbles and MisRob February 22, 2023 08:23
Copy link
Contributor

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, @Akila-I. Code changes are looking good to me.

There's one more step that is needed to fix the codebase and that is running npm run lint-frontend:format (automatical fix) and then committing all remaining files (those will be files that didn't need manual fix). I think this should also solve the failing linting check on this pull request.

@MisRob
Copy link
Contributor

MisRob commented Feb 22, 2023

And just a side note, there's no need to be concerned about the "Kolibri Build Assets for Pull Request / Build DMG file / build_dmg (pull_request) " failing check, it's not related to this PR

@github-actions github-actions bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) labels Feb 22, 2023
@Akila-I
Copy link
Contributor Author

Akila-I commented Feb 22, 2023

Hi @MisRob , changes you requested are made and pushed.

@MisRob MisRob self-requested a review February 22, 2023 16:57
@MisRob
Copy link
Contributor

MisRob commented Feb 22, 2023

@Akila-I Thank you,I'll have a look soon

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

All changes look correct here!

Please feel free to push another commit here, or add a follow up PR adding yourself to the AUTHORS.md file.

@rtibbles rtibbles linked an issue Feb 22, 2023 that may be closed by this pull request
@github-actions github-actions bot added DEV: backend Python, databases, networking, filesystem... SIZE: medium labels Feb 23, 2023
@Akila-I
Copy link
Contributor Author

Akila-I commented Feb 23, 2023

AUTHORS.md updated. sorry had to force push again because I had not have my fork synced with upstream earlier. thanks for the opportunity @rtibbles @MisRob . Looking forward to contribute more to Kolibri.

@github-actions github-actions bot removed the DEV: backend Python, databases, networking, filesystem... label Feb 23, 2023
@github-actions github-actions bot removed APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) labels Feb 23, 2023
@github-actions github-actions bot added the APP: Learn Re: Learn App (content, quizzes, lessons, etc.) label Feb 23, 2023
@github-actions github-actions bot added the APP: User Re: User app (sign-in, sign-up, user profile, etc.) label Feb 23, 2023
Copy link
Contributor

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @Akila-I and for reviewing @rtibbles. I briefly skimmed through the most recent force push and haven't noticed anything suspicious, so I think it's ready for merge. I will wait for @rtibbles to do so because I'm not sure what would be the best timing in regard to one of his linting PRs.

@Akila-I
Copy link
Contributor Author

Akila-I commented Feb 23, 2023

Thanks for the guidance and quick responses @MisRob, If this issue is finished, would love to take part in another contribution. Any suggestions of issues?

@rtibbles rtibbles merged commit ae74662 into learningequality:develop Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) SIZE: medium SIZE: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add prefer-const linting rule to kolibri-tools
3 participants