denoland / deno_lint

Blazing fast linter for JavaScript and TypeScript written in Rust
https://lint.deno.land/
MIT License
1.53k stars 172 forks source link

feat: add explanation for ignore directives #1229

Closed chengr4 closed 8 months ago

chengr4 commented 8 months ago

Hi,

This is my first contribute in deno codebase. I feel nervous and hope it can go well.

This PR is for #1076.

Please check my updates and slowly guide me to the right direction 🙏. Thanks in advance

TODO

CLAassistant commented 8 months ago

CLA assistant check
All committers have signed the CLA.

bartlomieju commented 8 months ago

Looks like a good start. Please run tools/format.ts to format the code and make CI happy.

chengr4 commented 8 months ago

hi @bartlomieju, I fixed CI and added test cases for test_parse_global_ignore_directives. Please check it out when you get time, and tell me anything I missed 🙏.

BTW, There are two things I have zero confidence.

  1. Variable Naming: eg. comment_text_without_reason and IGNORE_COMMENT_REASON_RE <= I hope these are ok.
  2. let comment_text =
        IGNORE_COMMENT_CODE_RE.replace_all(&comment_text_without_reason, ",");

    About adding & in front of comment_text_without_reason, I only followed the instruction of the compiler and hope it was the best practice.

bartlomieju commented 8 months ago

hi @bartlomieju, I fixed CI and added test cases for test_parse_global_ignore_directives. Please check it out when you get time, and tell me anything I missed 🙏.

BTW, There are two things I have zero confidence.

  1. Variable Naming: eg. comment_text_without_reason and IGNORE_COMMENT_REASON_RE <= I hope these are ok.
let comment_text =
        IGNORE_COMMENT_CODE_RE.replace_all(&comment_text_without_reason, ",");

About adding & in front of comment_text_without_reason, I only followed the instruction of the compiler and hope it was the best practice.

Yeah, these seems fine 👍

chengr4 commented 8 months ago

Sorry, I pressed the button accidentally. I didn't intend to rush you 😓.