Add linter CI #1301
Add linter CI #1301
Conversation
mengweieric
force-pushed
the
feature/CI-linting
branch
from
8abd49a
to
5c2f358
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1301 +/- ##
=======================================
Coverage 56.32% 56.32%
=======================================
Files 327 327
Lines 11944 11944
Branches 2741 2741
=======================================
Hits 6728 6728
Misses 5171 5171
Partials 45 45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Hi @mengweieric, thanks for putting this together. I just left some comments
mengweieric
requested review from
pjfitzgibbons,
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
rupal-bq,
vamsi-amazon,
Swiddis,
penghuo,
seankao-az,
anirudha,
paulstn and
sumukhswamy
as code owners
| working-directory: OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }} | ||
| run: | | ||
| git fetch origin main | ||
| CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB origin/main | grep -E "\.(js|ts|tsx)$") |
There was a problem hiding this comment.
why --diff-filter? seems unnecessary
There was a problem hiding this comment.
This is for not including files that are deleted, and also with this it explicitly shows what are included and what are not. I actually tried before to run CI without these flags but it was getting errors, and it runs successfully after adding it back. without flags: https://github.com/mengweieric/dashboards-observability/actions/runs/7254224280/job/19762669894. Do you have any other suggestions that it can be written differently without those flags but do the work?
There was a problem hiding this comment.
if you just want to exclude deleted, it should be --diff-filter=d?
also you are right it'll cause issues with deleted files
There was a problem hiding this comment.
The --diff-filter we specify here are actually specifying what we select from git diff if I'm not mistaken: https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---diff-filterACDMRTUXB82308203, therefore here without 'D' we skipped deleted files but select all others.
mengweieric
force-pushed
the
feature/CI-linting
branch
3 times, most recently
from
7e6a421
to
84f4cc1
Compare
mengweieric
force-pushed
the
feature/CI-linting
branch
from
3ebf9e4
to
280a8d7
Compare
Signed-off-by: Eric <menwe@amazon.com>
* linter CI (#1301) Signed-off-by: Eric <menwe@amazon.com> * change version to 2.x Signed-off-by: Eric <menwe@amazon.com> * add missing scripts Signed-off-by: Eric <menwe@amazon.com> --------- Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
* linter CI (opensearch-project#1301) Signed-off-by: Eric <menwe@amazon.com> * change version to 2.x Signed-off-by: Eric <menwe@amazon.com> * add missing scripts Signed-off-by: Eric <menwe@amazon.com> --------- Signed-off-by: Eric <menwe@amazon.com> (cherry picked from commit 138bd21)
Description
Add linter CI to lint only changed files including
The linting will be ran only on changes matching above patterns in that PR.
Sample linting run result
https://github.com/mengweieric/dashboards-observability/actions/runs/7215956806/job/19661217812#step:8:1
Sample annotation when linting errors found
mengweieric@3052873#diff-2790e5916efcabbaa1ad67efe53928cc72a31ecdf3094cfd0c9582471f91530eL85
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.