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

PR Workflow Enhancement - Delta Error/Warning Checks #1117

Closed
3 tasks done
tasneemkoushar opened this issue Dec 3, 2023 · 27 comments
Closed
3 tasks done

PR Workflow Enhancement - Delta Error/Warning Checks #1117

tasneemkoushar opened this issue Dec 3, 2023 · 27 comments
Assignees

Comments

@tasneemkoushar
Copy link
Contributor

Description
Currently, our PR Workflow checks the entire codebase for linting, formatting, and type errors, which can be time-consuming. This enhancement request is to optimize the workflow to only check for errors and warnings in the lines of code that are changed or added in the pull request.

Details:

Linting: Only check for linting errors and warnings in the modified or added lines of code.
Formatting: Run formatting checks only on the changes introduced in the pull request.
Type Errors: Optimize the type-checking process to focus on the modified or added code.
Context

This is the required check - PR Workflow / Check for linting, formatting and type errors (pull_request)
This enhancement aims to speed up the PR review process by narrowing down the scope of checks to the code that is directly affected by the pull request. It helps in quicker identification and resolution of issues related to the changes introduced in the PR.
Expected Behavior
The PR Workflow should only check for linting, formatting, and type errors in the modified or added lines of code.
The workflow should be optimized to reduce processing time for pull requests.

Additional Information

  • I have reviewed the current PR Workflow configuration.
  • I have considered potential impacts on existing workflows.
  • This enhancement aligns with our project goals.
@skbhagat0502
Copy link
Contributor

@tasneemkoushar Please assign me this issue.

@noman2002
Copy link
Member

Our policy is to assign no more than two issues to each contributor across all repositories. This way everyone gets a chance to participate in the projects. We sometimes give exceptions for more urgent cases and sometimes we lose track, but the policy stands. You have reached your limit, please wait until your existing issues are closed before requesting more issues. You could unassign yourself from one of the other issues too.

@duplixx
Copy link
Member

duplixx commented Dec 4, 2023

@noman2002 i wanna contribute on this issue..please asssign me this
thanks !!

@tasneemkoushar
Copy link
Contributor Author

sure @duplixx go ahead. But take this on priority

@duplixx
Copy link
Member

duplixx commented Dec 5, 2023

yeah sure @tasneemkoushar i'll finish it on priority basis
thanks!!

@duplixx
Copy link
Member

duplixx commented Dec 6, 2023

Hi @tasneemkoushar,

I've made updates to the PR workflow to specifically check only the changed files rather than the entire repository. Could you please review it so that I can proceed to open the PR?

image

@duplixx duplixx mentioned this issue Dec 7, 2023
@duplixx
Copy link
Member

duplixx commented Dec 11, 2023

already fixed the issue and merged can you please close it

@palisadoes palisadoes reopened this Dec 12, 2023
@palisadoes
Copy link
Contributor

palisadoes commented Dec 12, 2023

@duplixx Please take a look. This needs to be resolved. It's a really valuable feature that would greatly improve our code quality.

Do you have any ideas as to why it's failing? It seems like GitHub may be using a different version of a dependency than what you may have installed on your system.

@duplixx
Copy link
Member

duplixx commented Dec 12, 2023

yeah i am fixing it will push by EOD

@duplixx
Copy link
Member

duplixx commented Dec 12, 2023

`name: PR Workflow
on:
pull_request:
branches:
- '**'

env:
CODECOV_UNIQUE_NAME: CODECOV_UNIQUE_NAME-${{ github.run_id }}-${{ github.run_number }}

jobs:
Continuous-Integration:
name: Performs linting, formatting, type-checking and testing on the application
runs-on: ubuntu-latest
steps:
- name: Checkout the Repository
uses: actions/checkout@v3

  - name: Install Dependencies
    run: npm install --legacy-peer-deps

  - name: Install TypeScript
    run: npm install -g typescript

  - name: Run linting check
    run: npm run lint:check

  - name: Count number of lines
    run: |
      chmod +x ./.github/workflows/countline.py
      ./.github/workflows/countline.py --lines 1000 --exclude_files src/screens/LoginPage/LoginPage.tsx

  - name: Check formatting
    run: npm run format:check

  - name: Check for type errors
    run: tsc --skipLibCheck

  - name: Run tests
    run: npm run test -- --watchAll=false --coverage

  - name: Present and Upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}}
    uses: codecov/codecov-action@v3
    with:
      verbose: true
      fail_ci_if_error: false
      name: '${{env.CODECOV_UNIQUE_NAME}}'

  - name: Test acceptable level of code coverage
    uses: VeryGoodOpenSource/very_good_coverage@v2
    with:
      path: "./coverage/lcov.info"
      min_coverage: 95.0

Graphql-Inspector:
name: Runs Introspection on the github talawa-api repo on the schema.graphql file
runs-on: ubuntu-latest
steps:
- name: Checkout the Repository
uses: actions/checkout@v3

  - name: Set up Node.js
    uses: actions/setup-node@v3
    with:
      node-version: '16.14.1'

  - name: Resolve dependency
    run: npm install -g @graphql-inspector/cli

  - name: Clone API repository
    run: git clone https://github.com/PalisadoesFoundation/talawa-api && ls -a

  - name: Validate Documents
    run: graphql-inspector validate './src/GraphQl/**/*.ts' './talawa-api/schema.graphql'

`

this should work now

This was referenced Dec 12, 2023
@duplixx
Copy link
Member

duplixx commented Dec 13, 2023

its merged please close this issue
thanks!!

@palisadoes
Copy link
Contributor

We have to revert the PR. It is not being applied to the deltas

@palisadoes palisadoes reopened this Dec 13, 2023
@duplixx
Copy link
Member

duplixx commented Dec 13, 2023

@palisadoes, I've made an attempt to address the issue, but unfortunately, I'm unable to resolve it. I would greatly appreciate assistance from the maintainers, especially @tasneemkoushar in resolving this.

@palisadoes
Copy link
Contributor

palisadoes commented Dec 13, 2023

@duplixx Could this work? Use the changed-files GitHub action to get a list of changed files and then use a BASH for loop to identify individual files to run the tsc against.

@palisadoes
Copy link
Contributor

@duplixx Any updates on this?

@duplixx
Copy link
Member

duplixx commented Dec 15, 2023

Hey @palisadoes, I am resolving it as soon as possible. I have read the reference you shared. I'll update you by EOD

@duplixx
Copy link
Member

duplixx commented Dec 16, 2023

@palisadoes I have added for loop for the changed file, Can you check it once..

image

@tasneemkoushar
Copy link
Contributor Author

@duplixx Yeah, the loop is fine, go ahead.

@tasneemkoushar
Copy link
Contributor Author

@duplixx I have texted you on Slack, please check.

@duplixx
Copy link
Member

duplixx commented Dec 16, 2023

Hello @tasneemkoushar @palisadoes,

I am proposing a solution to enhance our linting checks and streamline the pull request (PR) workflow. My suggestion is to leverage Husky for this purpose.

When we conduct linting checks on selected files through GitHub Actions, it can consume a significant amount of resources and time. Moreover, there's a risk of exceeding the actions time limit. To mitigate these challenges, I propose incorporating Husky into our workflow.

Husky is a tool that automates tasks like checking for ESLint and Prettier errors. It executes these checks on precommit, ensuring that errors are addressed immediately. This approach not only prevents the introduction of errors into the codebase but also fixes or flags lint and Prettier issues directly in developers' terminals.

By implementing Husky in our workflow, we can proactively identify and resolve issues during the development phase, avoiding disruptions to the PR workflow and ensuring a more efficient and error-free coding process.

@palisadoes
Copy link
Contributor

  1. I'll leave that up to you and @tasneemkoushar
  2. We are already using husky in the API and/or Admin
  3. We'll still need to check the delta files in the PR in case people bypass the husky based CLI. So think of ways in which this could be done.
  4. Remember in most cases we are editing only a few files at a time. So the for loop won't be a huge burden. Even so, think of ways to optimize the GitHub action to achieve the goal of checking the changed files

@duplixx
Copy link
Member

duplixx commented Dec 17, 2023

It should work now this is solution i got from documentations and many organizations are using this way

image

image

@duplixx
Copy link
Member

duplixx commented Dec 17, 2023

@tasneemkoushar can we check it now if this works

@palisadoes
Copy link
Contributor

palisadoes commented Dec 17, 2023

@duplixx Please work with @anwersayeed on this for the next week as @tasneemkoushar will be unavailable. Contact him on Slack to coordinate the testing.

@duplixx
Copy link
Member

duplixx commented Dec 17, 2023

sure !!

@duplixx
Copy link
Member

duplixx commented Dec 19, 2023

@anwersayeed can you check the pull request i have raised regarding updated workflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants