Skip to content

apply new rules and update dependencies#202

Merged
le-cong merged 3 commits intomainfrom
apply-new-rule-require-service-call-response-declaration
Feb 5, 2025
Merged

apply new rules and update dependencies#202
le-cong merged 3 commits intomainfrom
apply-new-rule-require-service-call-response-declaration

Conversation

@le-cong
Copy link
Contributor

@le-cong le-cong commented Feb 4, 2025

the new rules which will be applied since the last release are:

Questions:

  1. regarding the new rule @checkdigit/no-status-code-assert, i assume it should be disabled in test codes right?
  2. regarding the new rule @checkdigit/require-service-call-response-declaration, it's turned off for test code as of now. the assumption is that maybe it's ok to call some services and assume the results are successful without asserting their response status codes, though someone can argue that it could introduce behavior changes to the tests hense this rule should be enabled for test code as well. i'm not so sure what's the right balance here ... thoughts?

@le-cong le-cong self-assigned this Feb 4, 2025
@le-cong le-cong added the MINOR label Feb 4, 2025
@github-actions
Copy link

github-actions bot commented Feb 5, 2025

Beta Published - Install Command: npm install @checkdigit/eslint-config@11.1.0-PR.202-5f5b

Copy link
Contributor

@carlansley carlansley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lftm. yes I believe those should be turned off for spec/test files, at least for now

@github-actions
Copy link

github-actions bot commented Feb 5, 2025

✅ PR review status - All reviews completed and approved!

@le-cong le-cong merged commit 0783c52 into main Feb 5, 2025
8 of 9 checks passed
@le-cong le-cong deleted the apply-new-rule-require-service-call-response-declaration branch February 5, 2025 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants