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

Integrating JShint into our Workflow #46

Merged
merged 3 commits into from
Oct 30, 2024
Merged

Integrating JShint into our Workflow #46

merged 3 commits into from
Oct 30, 2024

Conversation

khalifat3
Copy link
Collaborator

@khalifat3 khalifat3 commented Oct 30, 2024

This pull request finalizes the integration of JSHint into our development workflow. The integration includes adjustments to our package.json, the addition of JSHint configuration files, and updates to our GitHub Actions to incorporate JSHint checks. This ensures that our code adheres to our specified coding standards automatically before merges.

  • Adding JSHint to scripts in package.json:
    Modified the scripts section to include a JSHint run command, allowing developers to execute lint checks manually.

  • Added and populated .jshintignore & .jshintrc:
    .jshintignore: Specified files and directories that JSHint should ignore during its run.
    .jshintrc: Defined a set of linting rules tailored to our project's coding standards to ensure consistency.

  • Trying to Integrate JSHint into our GitHub workflow:
    Updated our GitHub Actions configuration to try and run JSHint checks on pushes and pull requests to the f24 branch. This is crucial for catching issues early and ensuring that only code that meets our quality standards is merged. This does not function correctly in this PR even though we did add the jshint.yaml, but we fixed it in Fixing the jshint.yaml to Run as Part of Workflows #48.

@coveralls
Copy link

coveralls commented Oct 30, 2024

Pull Request Test Coverage Report for Build 11589677579

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 30 of 33 (90.91%) changed or added relevant lines in 22 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.006%) to 82.686%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/meta/configs.js 0 1 0.0%
src/socket.io/categories.js 0 1 0.0%
src/socket.io/modules.js 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
src/user/auth.js 1 92.06%
Totals Coverage Status
Change from base Build 11555126436: -0.006%
Covered Lines: 22351
Relevant Lines: 25612

💛 - Coveralls

@Abdallah-Abdaljalil Abdallah-Abdaljalil changed the title Integrating jshint into our workflow Integrating JShint into our Workflow Oct 30, 2024
@khalifat3 khalifat3 merged commit 4c2ff42 into f24 Oct 30, 2024
1 check passed
@khalifat3 khalifat3 deleted the jshintintegration branch October 30, 2024 08:45
@khalifat3 khalifat3 restored the jshintintegration branch October 30, 2024 09:03
@khalifat3 khalifat3 added the Tools Installation installing tools needed to enhance development label Oct 30, 2024
@Abdallah-Abdaljalil
Copy link
Collaborator

Abdallah-Abdaljalil commented Oct 30, 2024

We made changes associated with integrating JShint into our workflow and fixing the errors JShint was giving us locally. They passed all the tests here in the PR when we committed our changes, however, we made the mistake of merging instantly instead of adding a reviewer. This is why we created PR #47 to revert the changes done by this PR. I am commenting on this PR to clarify the lack of a review before the merge. I would have said it was fine to merge anyway since I saw that all the tests passed both locally and on GitHub before the merge mistake. We also created PR #48 because we thought it would be safer to create a new branch for any additional changes we need to make. We forgot to modify the jshint.yaml file so that it runs as part of our workflows on GitHub. That's the reason for the extra branch and PR.

@Abdallah-Abdaljalil Abdallah-Abdaljalil added this to the Project 3B milestone Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tools Installation installing tools needed to enhance development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants