Added Github action that runs eslint on pull request

[aysp-99]

Fixes #100

Added Github Action to run eslint on pull requests and displays result using annotations-

• used this approach

• created linter.yml file in /github/workflows folder to create eslint action

• runs eslint on pull request and displays annotations for errors

[anandbaburajan]

Thanks @aysp-99! Can you push a code change with ESLint warnings just to test? Like remove a line break at the end of a file? Also use 'Fixes' instead of 'Resloves' in the first comment to let Github know which issue this PR fixes. I hope you're following the discussions at https://github.com/RocketMeet/RocketMeet-mailer/pull/23 too.

[aysp-99]

Thanks @aysp-99! Can you push a code change with ESLint warnings just to test? Like remove a line break at the end of a file? Also use 'Fixes' instead of 'Resloves' in the first comment to let Github know which issue this PR fixes. I hope you're following the discussions at RocketMeet/RocketMeet-mailer#23 too.

Yes, I am following the discussions. Regarding testing the Github action, I created a fork of my RocketMeet-client repository using second account and apparently it is not working. It is not displaying any warnings. Should I make this as a draft pr, so that I can resolve the issues?

[anandbaburajan]

I created a fork of my RocketMeet-client repository using second account.

You can create another test branch at your forked repo and open a PR there too.

Should I make this as a draft pr, so that I can resolve the issues?

That's not necessary. You can just keep pushing commits.

[aysp-99]

You can create another test branch at your forked repo and open a PR there too.

Actually, as mentioned in the original repository of reviewdog there is a limitation regarding displaying error messages when a pull request is created from forked repository. Due to which, I thought to create a fork of my Github repository from a second account to test if it works or not.

Though, I created a branch and tested by creating a pull request but it doesn't seem to work I don't understand why. Here's the screen shot of it:

That's not necessary. You can just keep pushing commits.

Okay

[anandbaburajan]

Actually, as mentioned in the original repository of reviewdog there is a limitation regarding displaying error messages when a pull request is created from forked repository.

Interesting! So it won't work even with a custom token? In that case we might have to go with a different solution. I think you and @mdpial should discuss this here or at our chatroom and find a solution together. :D

[piall]

I am trying this approach. If you are trying other please share with me. I would also like to try that. @aysp-99

[aysp-99]

Interesting! So it won't work even with a custom token?

For me it is not even showing errors on pull request created by another branch which I created.

[aysp-99]

I am trying this approach. If you are trying other please share with me. I would also like to try that. @aysp-99

Sure. I am still trying to fix it.

[anandbaburajan]

@aysp-99 You might wanna go with the approach used at the server and mailer. You can check out the comments at those PRs for 'why'. Thanks for persevering! :)

[aysp-99]

@aysp-99 You might wanna go with the approach used at the server and mailer. You can check out the comments at those PRs for 'why'. Thanks for persevering! :)

Yes, sure. Will do it! Just wanted to check one last time if it displays errors as comments/annotations on PR. Thanks.

[aysp-99]

@aysp-99 You might wanna go with the approach used at the server and mailer. You can check out the comments at those PRs for 'why'. Thanks for persevering! :)

Yes, sure. Will do it! Just wanted to check one last time if it displays errors as comments/annotations on PR. Thanks.

Done. Though, it only runs eslint on src folder.

[anandbaburajan]

Just wanted to check one last time if it displays errors as comments/annotations on PR.

Seems like there are no good solutions available for that, so we'll settle with this one! Btw, in case you didn't notice, the check comments appear here (once it's working).

Done. Though, it only runs eslint on src folder.

Oh, did you find any option to run it on all *.ts files or something like that? Also, if the checker was working properly, it should have shown the results here, like on @mdpial's PRs. Can you take a look? You can refer this too!

[piall]

@anandbaburajan Sorry for this mistake 😞 . After reading all the comments I have tried to make an ESLint error outside of the src folder and saw don't consider as error. I have looked for a solution and found. Should I create another PR for the mailer and server? You don't have to consider these PRs part of GSSoC as it was my mistake I didn't notice and once already considered. 😓

@aysp-99 You can try this glob pattern "./**". It will run ESLint from the root folder. If you want to use my version add after uses: icrawl/action-eslint@v1 line 😃

+        with:
+        custom-glob: "./**"

Updates: This approach has drawbacks. It tries to lint others files too.
May be it would be good to write code to lint separately folders 🤔

[anandbaburajan]

This approach has drawbacks. It tries to lint others files too.

We should only lint .ts and .tsx files actually (at server and mailer too) as mentioned in the readme because our config is setup for Typescript and we're also following the airbnb-typescript style guide.

Should I create another PR for the mailer and server?

Sure.

[aysp-99]

Seems like there are no good solutions available for that, so we'll settle with this one! Btw, in case you didn't notice, the check comments appear here (once it's working).

Got it!

Also, if the checker was working properly, it should have shown the results here, like on @mdpial's PRs. Can you take a look? You can refer this too!

I am still not able to figure out why isn't it displaying the errors as annotations. 😕

[aysp-99]

Oh, did you find any option to run it on all *.ts files or something like that?

Updates: This approach has drawbacks. It tries to lint others files too.
May be it would be good to write code to lint separately folders 🤔

I tried using this: "." and it runs EsLint on .ts and .tsx files from the root folder, though it doesn't display some EsLint errors as annotations. I created a pull request with changes in pages/app.tsx, pages/index.tsx, src/components/Footer.tsx, src/components/Login.tsx and src/store/store.ts. It only shows annotations for pages/app.tsx and pages/index.tsx but it shows the errors in EsLint logs for changes made in the other files.
Let me know @mdpial if you've tried any other glob pattern that works.

[piall]

@aysp-99 . wont to go to nested folders. You need to use ./** for that. After trying lots of patterns and other things I have found this glob pattern works fine ./**/*.ts{,x} for me (checks only ts & tsx). I have tested in my forked repository by adding lots of test scenarios. You can check those.
Here is the link of test PR in my forked repository. In that test PR I have added comment for each test and also the action link for each particular test.

[aysp-99]

@aysp-99 . wont to go to nested folders. You need to use ./** for that. After trying lots of patterns and other things I have found this glob pattern works fine ./**/*.ts{,x} for me (checks only ts & tsx). I have tested in my forked repository by adding lots of test scenarios. You can check those.
Here is the link of test PR in my forked repository. In that test PR I have added comment for each test and also the action link for each particular test.

Thanks! I used "**/*.{ts,tsx}" and it works. I created a pull request for testing it. It displays annotations for EsLint errors of commits.

[anandbaburajan]

We should only lint .ts and .tsx files actually (at server and mailer too) as mentioned in the readme because our config is setup for Typescript and we're also following the airbnb-typescript style guide.

Sorry for the confusion: only .ts files needed for server and mailer. But both .ts and .tsx for the client! Thank you two for the efforts.

[anandbaburajan]

Thank you @aysp-99! I've put one comment, PTAL. I've a doubt, does anyone know why the action isn't running here but it ran at @mdpial's PRs?

[anandbaburajan]

Thanks @aysp-99! You can remove the dummy errors and I'll merge! After the PR is merged, please open a test PR with dummy errors just to see if the action works as expected.

[aysp-99]

Thanks @aysp-99! You can remove the dummy errors and I'll merge!

Done.

After the PR is merged, please open a test PR with dummy errors just to see if the action works as expected.

Sure. Will do it.

[anandbaburajan]

Sure. Will do it.

Hey, no need - it works! :D Check #116.