feat: ignore .md files when do requiresJenkinsRun check#641
feat: ignore .md files when do requiresJenkinsRun check#641aduh95 merged 1 commit intonodejs:mainfrom
requiresJenkinsRun check#641Conversation
Codecov Report
@@ Coverage Diff @@
## main #641 +/- ##
=======================================
Coverage 84.09% 84.09%
=======================================
Files 37 37
Lines 4074 4081 +7
=======================================
+ Hits 3426 3432 +6
- Misses 648 649 +1
Continue to review full report at Codecov.
|
|
covert to draft because I still need to add more tests for this feature to keep coverage increasing. |
494fed4 to
662b459
Compare
tniessen
left a comment
There was a problem hiding this comment.
How often does this help, i.e., how often do PRs edit *.md files outside of doc?
If it does not happen a lot, I am concerned about being too permissive.
- There is no guarantee that dependencies will use
.mdfor documentation only. - Even node itself does not use
.mdfor documentation only. For example,test/fixturescontains.mdfiles that affect tests (e.g., because they are linter test cases).
@tniessen those tests are covered by the GHA CI, I think it's safe to skip Jenkins CI for those.
Sure, but that seems even more rare than what this PR is trying to fix. For those case, it's still possible to add the |
I don't think it's a problem with the current state of the repository, but there is no guarantee that this won't change in the future.
Agreed, both false positives without this PR and false negatives with this PR are unlikely. Anyway, I am not explicitly blocking this PR; erroring on the side of caution is merely my personal preference. |
@tniessen Node core will add This patch is useful in the below scenario:
This patch's motivation is to remove this surprise: If you do think this |
This pr ignores .md files when do
requiresJenkinsRuncheckRefs: nodejs/node#43483 (comment)
cc @aduh95