-
Notifications
You must be signed in to change notification settings - Fork 596
[Feat] Introduce linting in frontend project #187
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
Conversation
@microsoft-github-policy-service agree |
@matheusmaldaner as promised, here you are! :) Would appreciate your through review as the PR is medium-sized due to fixing the existing linting error. |
de84370
to
2ec2323
Compare
Hi @stefanoamorelli, thank you so much for all the changes! Could you review my minor commit? Your linting implementation highlighted some other cases that were failing with the Do you know why the Frontend Checks (frontend-lint) are failing in the GitHub CI workflow? I ran the code on my end and there are no errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good linting changes, do not seem to change anything that could break the code. All tests (poe check
) are still passing.
@matheusmaldaner good stuff! I've reviewed your commit, LGTM left just a couple of comments to confirm. Also, I've pushed a commit to allow warnings to allow the CI/CD to pass with warnings (given that we don't introduce more warnings). |
Requested a review from @husseinmozannar on this so we can merge it before too many changes are made to the system and also so that he can maybe shed light into why the first check is still failing |
Sounds good 👌 |
Co-authored-by: matheusmaldaner <t-mkunzler@microsoft.com>
microsoft#185) Co-authored-by: 侯瑞 <hourui@xiaohongshu.com> Co-authored-by: Hussein Mozannar <hssein.mzannar@gmail.com>
2c4c1ac
to
7cc2bcc
Compare
7cc2bcc
to
9d66c14
Compare
@matheusmaldaner I've rebased the branch and pushed another commit to try fix the failing linting on CI. Is there anything I can do to help unblock this? Happy to help! |
Hi @stefanoamorelli the first check still fails and I am unsure why :/ Could you look into this whenever you got the time or let me know if there is anything I can do on my end to help us solve this issue? Thank you a lot for the contributions and the help with this |
Co-authored-by: Tyler Payne <tylerpayne@microsoft.com> Co-authored-by: Hussein Mozannar <hssein.mzannar@gmail.com> Co-authored-by: Hussein Mozannar <hmozannar@microsoft.com>
Co-authored-by: Hussein Mozannar <hmozannar@microsoft.com> Co-authored-by: Hussein Mozannar <hssein.mzannar@gmail.com>
Co-authored-by: Tyler Payne <tylerpayne@microsoft.com> Co-authored-by: Hussein Mozannar <hmozannar@microsoft.com>
Co-authored-by: Tyler Payne <tylerpayne@microsoft.com>
Co-authored-by: Tyler Payne <tylerpayne@microsoft.com> Co-authored-by: Hussein Mozannar <hssein.mzannar@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: matheusmaldaner <t-mkunzler@microsoft.com>
Fix playwright fill id to use coordinates rather than element typing. Fix already covered by playwright tests Better simulates how real users would interact with the website by sending actual keyboard events to the browser
Co-authored-by: Hussein Mozannar <hssein.mzannar@gmail.com> Co-authored-by: Hussein Mozannar <hmozannar@microsoft.com>
8c7389a
to
6b5c9c6
Compare
@matheusmaldaner thank you! The branch was outdated and I must have corrupted the git history when resolving all the conflicts (there were quite a few as the linting fixes touched a lot of files). I've created a new PR with the most up to date branch and the linting should pass there (it was failing because new errors appeared when rebasing): #234 I'm closing this one and we can continue the conversation in the new PR 🙏 (I've also split the changes in atomic commits as an effort to make review easier for you!) |
Solves: #133
We introduce
ESlint
for thefrontend/
, fix the existing issues, and add aGithub
workflow to run the linting and prevent merging of code that does not pass the linting.